Close the deferred hardening notes from the review round - #76
Merged
Conversation
Six items the #74/#75 reviewers classified low/informational -- nothing here is reachable in a shipped build today; each is a seam a future change could silently reopen. privacy.sh's PCH_TCC_DB_PATH test seam now opens only under PCH_TEST_MODE=1, matching PCH_TEST_BREW_BIN and PCH_TEST_OSASCRIPT_BIN. It was the one injection point in the new modules without the gate, and ungated it doubles as an arbitrary-sqlite-read redirect for any future caller that forwards the variable. login_items.sh now refuses a manifest whose protocol version differs from its own -- fields this version reads could mean something else under another one. cleanup.sh cross-checks its whole manifest; this is the minimum equivalent. The bundled-runtime completeness guard now also covers non-.sh runtime dependencies (rules/*.json, data/*.json, report_i18n, *.jxa.js, and shipped .py), with an explicit build/reference-only exclusion list mirroring the .sh guard's. Proven the same way: removing rules/network.json from RUNTIME_FILES trips it. RuntimeWorkspace's bundled branch now demands the development opt-in whenever the injected resourceURL is not the running app's own signed bundle. That combination skips the code-signature payload comparison and is constructible only through parameter injection; without the gate, a future refactor decoupling the parameters would run unsigned code with no test noticing. The revalidation tests declare the opt-in they were implicitly relying on. Raw pinned-file values now refuse on key collision instead of silently clobbering whichever module payload merged first -- the exact asymmetry behind the approval_token/approval_token_module near-miss fixed in #67. Observation timeout slack grows from 20s to 60s past the window (the snapshots run exactly when the machine is busiest), and a timeout now says it timed out instead of "status 124".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The six low/informational items deferred from #74/#75. Nothing here is reachable in a shipped build today — each is a seam a future change could silently reopen, so each gets closed plus (where testable) a guard with proven teeth.
1.
privacy.sh—PCH_TCC_DB_PATHgated behindPCH_TEST_MODE=1The one test-injection seam in the new modules without the gate (
PCH_TEST_BREW_BINandPCH_TEST_OSASCRIPT_BINboth have it). Ungated, any future caller forwarding the variable turns it into an arbitrary-sqlite-read redirect. The existing TCC fixture test now declares the mode.2.
login_items.sh— manifest protocol-version check at executeFields this version reads could mean something else under another protocol version.
cleanup.shcross-checks its whole manifest; this is the minimum equivalent. New test tamperingversion 1 → 2provesblocked— and fails against the pre-fix script.3. Bundle-completeness guard extended beyond
.sh#71's guard only globbed
scripts/**/*.sh, so a future rules/i18n/data/JXA/runtime-py dependency could recreate the exact "CI passes from the checkout, signed bundle silently lacks it" incident. Nowrules/*.json,data/*.json(minus the gitignored local config),data/report_i18n/*.json,scripts/*.jxa.js, and shipped.pyare all required inRUNTIME_FILES, with an explicit build/reference-only exclusion list mirroring the.shguard. Teeth proven by removingrules/network.jsonfrom the manifest — the guard trips with the exact filename.4.
RuntimeWorkspace— foreign-bundle branch requires the development opt-inThe bundled branch skips
runtimePayloadMatchesCodeSignaturewhenever the injectedresourceURLisn't the running app's own bundle — a combination constructible only through parameter injection, but nothing stopped a future refactor from decoupling the parameters and silently running unsigned code. It now demandsPCH_DEVELOPMENT_MODE=1, same as the development branch. The existing revalidation tests declare the opt-in they were implicitly relying on (the three refusal tests keep it too, so they still fail for the tampering under test rather than the missing opt-in). New test: foreign bundle without opt-in →nil, failing against the pre-fix code.5. Pinned-file raw values refuse on key collision
Module payloads merge keep-first, but raw values (the approval token) landed via unconditional overwrite — the exact asymmetry behind the
approval_token/approval_token_modulenear-miss found in #67's development. Both call sites now fail closed with an explicit internal-error message instead of clobbering whichever payload merged first. (No dedicated test: the collision is unconstructible from outside the function today — that unreachability is the point of the guard.)6. Observation timeout
Slack grows 20s → 60s past the window (the 4 lsof + 2 ps snapshots run exactly when the machine is busiest — the machines this feature targets), and
.timedOutnow reports "제한 시간을 넘겨 중단되었습니다" instead of the generic "status 124".Test plan
pytest tests/— 370 passedswift test— 169 passedrelease_smoke.py --check-only— okFollows #74/#75.
🤖 Generated with Claude Code