security(ci): acquire exact pull head as data - #16
Conversation
|
Post-CI review completed at
Design decision: acquire one fixed numbered PR ref into a fresh bare database, require the sole private ref to equal the expected full OID, then delegate object identity and path/mode admission to the existing reader. No checkout or target execution is introduced. Main risk: the repository-size check is post-fetch, not a hard transport or peak-disk quota. Git/HTTPS remain trusted parsers and scratch capacity is a caller precondition. The real PR #15 probe also exposed a separate current-repository compatibility blocker: eight paths are outside reader admission, so the end-to-end acquisition correctly refuses rather than yielding a partial snapshot. Compatibility impact: additive unwired API only. Existing scanner, baseline, reports, workflow events, permissions, caches, secrets, and repository settings are unchanged. No private-repository credential flow is added. Rollback path: revert the four commits or remove the helper/tests/docs. The reader and materializer remain independently available; there is no persisted format or settings migration. No blocking defect was found in the acquisition diff itself. Keep draft until delayed unchanged-head review on or after 2026-09-10. Do not activate the advisory workflow until path admission has a separate reviewed decision. |
cc99fa2 to
9668476
Compare
|
Builder remediation is complete at
Design decision: close only the ambient askpass credential channel; do not redesign acquisition. Main risk: the existing 64 MiB limit remains post-fetch rather than a hard transport/peak-disk cap. Compatibility impact: none outside child-Git credential isolation; the PR remains an additive, unwired API. Rollback: revert PR #16 remains draft for independent unchanged-head review. No workflow was activated. |
Summary
Add
acquire_pull_snapshot(remote, pull_number, expected_head_oid, scratch_root)as the independent pull-head acquisition boundary for Issue #10. The helper creates a fresh verifier-owned bare Git database, fetches exactly the numbered pull-request head ref, requires its tip to equal the expected full OID, and invokes the bounded reader before yielding.The PR has completed delayed security review. Its own delta against current
mainis 3 files / 617 additions: the acquisition helper, focused contract tests, and boundary documentation.Design decision
Keep acquisition verifier-owned and checkout-free. Accept only credential-free HTTPS remotes, plus caller-controlled
Pathfixtures; initialize from an empty template; fetch one explicit depth-one force refspec; import no tags or submodules; write noFETCH_HEAD; and require the fresh database to contain onlyrefs/repo-sentinel/acquired-headat the expected full lowercase SHA-1 or SHA-256 OID.Inherited
GIT_*state and global/system Git configuration are excluded. Terminal prompting is disabled,GIT_ASKPASSandSSH_ASKPASSare neutralized, and inheritedSSH_ASKPASS_REQUIREis removed. Replacement lookup, redirects, lazy fetch, automatic maintenance, and protocol restrictions remain unchanged. The reader remains authoritative for raw commit, tree, and blob identities, logical path admission, modes, and resource bounds. Acquisition does not rewrite paths or convert downstream refusal into success.Validation evolution
Before the path-contract merge, the live PR #15 probe acquired the exact expected head and reached the reader, which refused
unsupported_pathfor eight legitimate names outside the former portable ASCII subset. That historical result remains useful evidence that downstream reader refusal propagated without a partial snapshot and that the temporary database was cleaned.After PR #17 merged at
d8e30ba019247a21b9d42e1c1d52900a1f1de623, the same exact-head boundary traversed acquisition, the logical-path reader, and the portable materializer successfully. A controlled invalid-UTF-8 fixture now preserves the acquisition-to-reader refusal and cleanup regression without depending on the obsolete ASCII restriction.The final remediation adds a real
git credential fillregression. With hostile parentGIT_ASKPASS,SSH_ASKPASS, andSSH_ASKPASS_REQUIRE=force, the old behavior invoked the marker helper twice and accepted dummy credentials. The corrected environment invokes it zero times and Git fails non-interactively with terminal prompts disabled. A fetch-level authentication challenge also failed closed with no snapshot, zero marker calls, and complete cleanup.Main remaining risk
The 64 MiB repository-size check runs after fetch. It fails an over-budget result closed, but it is an admission bound after transport rather than a hard transport or peak-disk quota. The trusted scratch owner must reserve sufficient capacity until the fetch returns or the 60-second acquisition timeout terminates the command. Git and its HTTPS transport remain trusted parsers, and the reader retains its separate timeout.
Compatibility impact
This is an additive, unwired API. Credential-bearing URLs, non-HTTPS network protocols, query or fragment URLs, partial or uppercase OIDs, missing or moved refs, excess repository bytes, and reader refusal fail closed. Ambient askpass helpers cannot convert credential-free acquisition into a private-repository credential flow.
Scope
Workflow activation remains unwired. Scanner invocation remains unwired. Existing events, reports, baseline semantics, caches, permissions, secrets, repository settings, and schemas are unchanged. The rebased PR contains no reader or materializer implementation delta against current
main; the only post-review production change is child-Git askpass environment isolation.Validation
Microsoft Windows 10.0.26200, Python3.14.3, Git2.55.0.windows.5.python -m unittest tests.test_repo_sentinel_acquire -v: 13 tests passed.python -m unittest discover -s tests -p 'test_*.py': 71 tests passed.6.6.87.2-microsoft-standard-WSL2, Python3.12.3, Git2.43.0: the credential-isolation regression passed.python -m pre_commit run --files docs/repo-sentinel-baseline-review.md scripts/repo_sentinel_acquire.py tests/test_repo_sentinel_acquire.py: all applicable hooks passed.python -m ruff check scripts/repo_sentinel_acquire.py tests/test_repo_sentinel_acquire.py: passed.git diff --checkpassed.https://github.com/stacknil/sec-writeups-public.git.refs/pull/15/head.2f7b7a9bef43715141086b0d79bacbe67a178288.960dc6c6496f1260f6fab74b64f26408024cd5fb.ad39acf89d0826ce2651ed14d12e143d7f0a8b5cb6b2eb219b46895688f45d0a.FETCH_HEAD, and was removed on exit. The materialized target was also removed on exit. Repository content was never executed.Rollback path
Revert this PR or remove the acquisition helper, tests, and documentation. The merged reader and materializer remain independently usable. There is no persisted output, workflow deployment, credential, permission, setting, baseline, or schema migration.
Workflow activation remains a separate reviewed change and is not part of this PR.