fix(provider): contain repo-file reads to a symlink-safe root (P1) - #36
Merged
Conversation
`builtin/repo-file` enforced PATH containment only (`cleanRel` / `underAnyRoot`
are string guards). `os.DirFS` is documented as not a security boundary and
`fs.Stat` follows links, so the stated invariant "never a fact from outside the
declared roots" was false in production: `cmd/assent/provider_host.go` handed the
builtin an `os.DirFS` over the merge request's HEAD tree.
Reproduced on the live `assent run --checkout` route, both forms:
- directory symlink `topics/evil -> <outside>` with `Roots:["topics"]`
-> state=resolved value=999
- file symlink at a wholly legitimate in-root path
`topics/prod/quota.yaml -> /abs/host/cluster-secrets.yaml`
-> state=resolved value=31337, and end-to-end an APPROVE for a change no
in-repo quota allows
Two layers, different guarantees:
- injection site: `checkoutFS` now returns `builtin.OpenRepoRoot(dir)`
(`os.OpenRoot` + `(*os.Root).FS()`) - a syscall-level boundary for every
consumer of that FS; the root is closed by `resolveRunFacts`, and a
`--checkout` that cannot be opened as a root is a hard error, not a silent
degrade.
- builtin: `classifyCandidate` Lstats every path component and refuses on any
symlink. This is the only layer that can protect the declared `roots` CLIP
(a link inside the FS root can still leave the roots), and it catches the
directory-symlink form under a bare `os.DirFS`, where an Lstat of the leaf
alone reports an ordinary regular file.
Refusal is `unavailable` with a contributor-readable reason naming the candidate
- same class and fail direction as the existing "anchor outside declared roots",
never the raw OS text (ADR-0012) - and it STOPS the walk-up rather than skipping
to a less-specific file, which would mask the escape behind an ordinary-looking
resolution. In-root symlinks are refused too: one rule, and it is the rule that
keeps the roots clip honest. No fixture in the tree is a symlink.
`LoadResourceOwnerMap` reads the same FS with no roots at all and decides who may
approve; it refuses a symlinked registry with an error (no client, so the owner
fact never resolves).
Records: D-129, REQ-E5-S07-03. Closes OQ-28 (introduced by PR #35).
Found while assessing the D-129 sibling. `loadResourceOwnerRegistry` tried `repoFS` BEFORE `client.FileAtRef(targetRef)`, and under `--checkout` `repoFS` is the merge request's own HEAD tree - so an MR could ship `governance/owners.yaml` naming its author as owner of the resource it is changing, and satisfy an `ownership` obligation with a registry it wrote itself. The ownership registry decides WHO MAY APPROVE, which makes it a decision input: it must load from the target ref like every other deciding document, while the MR branch supplies only the material under judgment (GUIDELINES safety invariant 3 / ADR-0015 §1). Order is now target ref -> (on error) checkout -> error. Reproduced by mutation: with the old order the loaded owner is "attacker", with the new order "team-payments", same inputs. Compat is preserved - a target ref with no registry still falls back to the checkout copy, and both sides missing stays an error rather than an empty map that would make every resource unowned. Nothing depended on the old precedence: the live resource-owner path had no cmd-level test at all before this commit. `loadResourceOwnerRegistry` now takes a one-method `refFilePort` rather than the whole `forgePort`, which is what makes the precedence unit-testable. Records: D-130, REQ-E5-S08-03.
D-129 changed a silent degrade into a hard error, and the claim was untested: `TestOpenRepoRoot/missing_dir_is_an_error` covers the helper, not the propagation out of `resolveRunFacts`. Nothing upstream catches it first - run.go's governed read and foldCheckout both tolerate a missing checkout - so without this the run would evaluate a provider-configured policy against an empty Facts map and never say why. Mutation-proven: swallowing the error (`repoFS = nil`) turns the test red with `facts=map[] resolvedAt=map[]`; probe reverted.
…link-containment # Conflicts: # CHANGELOG.md
loadResourceOwnerRegistry fell back to the merge request's own head tree
on EVERY FileAtRef error, not only on absence. A forge that merely failed
-- 401, 429, 500, 502, 503, an expired token, a proxy hiccup -- therefore
handed the who-may-approve registry to the contributor, with no error
surfaced and nothing in the decision to show it happened. Reproduced on
the modify-plus-transient-error shape (a plain value diff on an existing
registry): a 503 resolves owner = "attacker". The 404 shape is separately
mitigated -- a whole-file registry add folds opaque -> REVIEW -- which is
why the modify shape was the live one.
That is a fail-OPEN under GUIDELINES Safety 2, and it falsified D-130's
own "never a shadow" claim.
The fallback is now gated on errors.Is(err, forge.ErrNotFound) -- the
NEUTRAL port sentinel in internal/forge/port.go, already wrapped with %w
by the GitLab adapter -- and deliberately not gitlab.ErrNotFound, which
would red the ARCH-02 / depguard boundary gate (D-123). Any other error
yields no client, so the owner fact never resolves and the run degrades
to REVIEW.
The three pre-existing registry stubs move from errors.New("404 file not
found") to the adapter's real wrapped shapes (absentAtRef / brokenForge).
This is not bending tests to pass: "any error opens the fallback" WAS the
defect, so a stub asserting it would have stayed green over the fix, and
the wrapped form additionally proves matching survives the wrap chain.
TestResourceOwnerRegistrySymlinkInCheckoutRefused's comment is corrected
in the same pass: it pins the CONJUNCTION of the two D-129 layers, not
the builtin guard. Verified by mutation -- green with the builtin guard
deleted (the root FS refuses at fs.Stat first), red only when the root FS
is also reverted to os.DirFS.
The leak loop scanned scalar(fact.Value) for 31337/999/4242/12, but the fatal immediately above it already pinned fact.Value to nil and scalar(nil) renders "null" -- the loop could never match, so it asserted nothing. Retargeted at fact.Reason, which is the seam that actually remains: the value is dropped on a refusal, but the reason is rendered into the posted MR thread, so it may name the refused candidate PATH and must never carry the refused file's CONTENT (ADR-0012 / the render.displayFactValue seam D-129 names). Mutation-proven: embedding the refused bytes in the builtin's refusal reason reds five subtests naming 999 / 31337 / 4242.
D-129 said the cmd/assent/checkout.go residual was bounded because
"filepath.WalkDir does not follow directory symlinks, so only a final-
component file symlink escapes there". That is materially false, and it
is the artifact a maintainer reads when deciding whether to tag. Replaced
with the two residuals that are actually live, severities stated plainly,
both being fixed in a sibling lane:
P0 -- collectTree (checkout.go:121) tolerates fs.ErrNotExist for the
ENTIRE WalkDir, not just a missing root, and WalkDir aborts on the first
callback error. A dangling symlink is Lstat'd as a non-directory, the
callback's os.ReadFile follows it and fails ENOENT, and the PARTIAL map
returns with NO error. WalkDir visits in lexical order, so a dangling
symlink named below 0x2E ("!evil", "-evil") erases .assent/** from the
changed-file set -- dominance class and opacity flag both. Driven end-
to-end through runRun --checkout, BLOCK becomes APPROVE, defeating the
D-042 self-vouch guard.
P1 -- readIfPresent (checkout.go:129) is a bare os.ReadFile, and
run.go:279 overrides the GOVERNED SUBJECT's base/head from it, so a
head-side file symlink makes an off-tree host file the document under
judgment. Confirmed APPROVE-manufacture and verbatim exfiltration into
the posted thread.
D-130's lede and body claimed "the checkout is a fallback, never a
shadow" while the code fell back on any forge error; amended in place to
"fallback ON ABSENCE ALONE" with the review finding recorded. Same for
REQ-E5-S08-03's parenthetical.
OQ-28 was cited as closed by D-129 and REQ-E5-S07-03 but no such row
existed (the table topped out at OQ-26) -- a dangling reference under
AGENTS.md rule 6. Added retroactively, stating the question as it was
raised and recording it closed, with the two checkout residuals carried
as its open residual.
The new row used a same-directory link (the convention every other row in that table follows), which resolves to docs/planning/decisions.md -- a file that does not exist. Nothing catches it: the DOC-05 link-resolution loop is scoped to README.md (D-124 (b)/(c)) and docs/planning/ is out of the mkdocs nav.
providerCallFor's builtin/resource-owner arm was correct but unentered by any cmd-level test: builtin/resource-owner appears in no config, example, exit gate or run test, so the providerCallFor -> loadResourceOwnerRegistry -> resolveRunFacts wiring had zero coverage. The loader-level tests pin the refusal; nothing pinned that the refusal actually leaves the host. Drives resolveRunFacts with the whole forge working except a 503 on the ownership registry read, over a checkout that carries an MR-authored registry. Mutation-proven red on both links (return nil, err -> return nil, nil in providerCallFor's resource-owner arm; the provider %q wrap -> continue in resolveRunFacts); no other test in the repo reds on either.
…t REVIEW D-130's amendment and REQ-E5-S08-03 both promised "the owner fact never resolves (fail-safe REVIEW)". Measured on the live runRun path with a forge that 503s only the registry read: exit code 1, nothing on stdout, the resolve providers error on stderr, and approvals=0 merges=0 discussionsPosted=0 notesPosted=0 notesUpdated=0. The error propagates loadResourceOwnerRegistry -> providerCallFor -> resolveRunFacts -> orchestrate, so the run aborts: no DecisionRecord, no thread, no forge write. Strictly more conservative than REVIEW, but the operational consequence differs and is now stated rather than buried — a flaky forge gives a red CI job with nothing in the merge request, not a reviewable thread. The same overreach in the lane's own test comment is corrected with it.
The declaration load at the head of resolveRunFacts' provider loop continues
on ANY FileAtRef error — the same absence/error conflation the D-130 amendment
closed for the registry — but it is fail-safe by a different mechanism.
Confirmed through runRun on the ownership policy: declaration present +
registry served from the target ref -> APPROVE; declaration 404 -> REVIEW
carrying effect: require-review, code: predicate.error, because a skipped
provider leaves the fact absent, the CEL bound reference errors, and the
engine routes a predicate error to require-review.
Documents a non-change, so no new D-row. Also names the residual: the skip is
silent (pins.factsResolvedAt is {}), so an operator cannot tell "provider not
declared" from "the forge failed to serve the declaration". Not fixed here.
…link-containment # Conflicts: # CHANGELOG.md # docs/decisions/decisions.md
…ner fixture D-130 said a non-absence forge error on the ownership-registry read produces an outcome "strictly MORE conservative than REVIEW". Measured with two runRun calls against one fakeGitLab: after an APPROVE run, the aborting rerun leaves that run's "Decision: APPROVE" summary note standing unrevised (notesUpdated=0) and posts no thread (discussionsPosted=0), whereas a REVIEW rerun upserts the same note to REVIEW (notesUpdated=1, note count 1) and posts one unresolved discussion. Arming refuses unless only_allow_merge_if_all_discussions_are_resolved is enabled (precondition.go -> ArmEligible -> ErrArmingRefused), so wherever assent can arm, that discussion is the merge gate and the abort supplies none. The row now separates what was measured from what was read, and names the residual as a stale-green risk on the human path rather than a UX gap. REQ-E5-S08-03 carried the same framing and is corrected in step; its symlink clause said the registry "is refused with an error (no client => the owner fact never resolves)" two clauses after stating that such errors abort the run — the same path, two described outcomes. Measured: a symlinked checkout registry with a 404 at the target ref returns an error out of resolveRunFacts, i.e. it aborts. resourceOwnerDeclarationJSON declared output `team` at maxAge 1h while builtin/resource-owner emits `owner` at 24h, so DeclarationsEqual synthesized a permanently invalid fact and D-130's "declaration present -> APPROVE" control did not discriminate a healthy resolve from a broken builtin. The fixture now mirrors builtin.OwnerDeclaration(), and TestResourceOwnerDeclarationResolvesOwnerFact pins the healthy resolve (mutation-proven red on the output name and the maxAge).
…and the origin/main merge
… what was measured
Both lanes added a requireSymlinks guard to package main, so the merged tree did not compile. Keep the probe-based definition (it verifies the capability rather than assuming it from GOOS) as the package-wide guard and drop the duplicate; the checkout-containment cases now share it.
…umeration refusal D-133 (merged as 71b573e after both reviewers approved this lane) refuses any symlink under base/ or head/ at changed-file enumeration, before providers resolve, so TestRunCheckoutSymlinkFactEscapeRefused's scenario is unreachable on the live path and its "the run must complete and DECIDE" assertion now asserts a behaviour the tool deliberately no longer has. The coverage is reshaped, not dropped. TestRunCheckoutSymlinkRefusedAtEnumeration asserts the subsumption plus the message contract, the no-exfiltration property and zero forge writes, and carries a tripwire: when ADR-0008 Amendment 2 folds the refusal opaque the exit code becomes 0, the case reds, and its message hands the engineer the successor by name. TestResolveRunFactsRefusesSymlinkedQuotaCandidate is that successor: it keeps the distinguishing assertion (a legitimate walk-up quota of 24 means skip-and-fallback would resolve, so only a refusal that stops the walk-up leaves the fact unresolved) at the deepest seam the live path still reaches - resolveRunFacts, the function runRun step 5c calls, through the sole production RepoFileOpts site. The absolute off-tree and relative in-root cases pin the two D-129 layers separately rather than as a conjunction gate.
…arming condition Records that D-129's live-path reproduction no longer re-runs on that route, that no alternative route exists (measured on both checkoutFS flat-fallback shapes: poisoned and clean trees emit byte-identical records), that the guard is now defence in depth rather than redundant, and the condition under which it becomes load-bearing again - ADR-0008 Amendment 2's fold-the-refusal-opaque direction. Also names what the reshaped tests pin versus the pre-existing cmd-edge pins, and the per-layer mutation proofs. No D-135: the guard's shape and fail direction are unchanged, only which layer fires first and where the proof lives.
…er D-133 OQ-28's notes column still said the cmd/assent/checkout.go residuals (collectTree truncation P0, readIfPresent governed-subject symlink P1) were open; D-133 fixed both and is merged into this lane, so the row was stale in the direction that understates the fix. The row now also states where the proof lives, because the honest reading changed: D-133 refuses any symlink under base//head/ at changed-file enumeration before providers resolve, so OQ-28's escape is no longer reproducible end-to-end through 'assent run --checkout'. REQ-E5-S07-03's Test/Verify pointers are corrected the same way - the old '-run TestRunCheckout' filter would have silently stopped covering the relocated proof.
…er case Guarding only the two poisoned subtests left the control subtest passing on a symlink-hostile runner, so the parent reported PASS over a containment proof that never ran - the exact silence requireSymlinks exists to prevent, and the contract the shared helper states in its own doc comment.
konih
added a commit
that referenced
this pull request
Aug 9, 2026
OQ-28's end-to-end escape is closed on main by D-133 (71b573e), which refuses any symlink under base/ or head/ at changed-file enumeration -- before providers resolve. The provider-layer guard (builtin.OpenRepoRoot syscall root plus classifyCandidate's per-component refusal, D-129) is defence in depth and lives in the dedicated provider lane, PR #36 at 1e71cec. Verified against origin/lane/fix-provider-symlink-containment rather than transcribed: OpenRepoRoot, the fs.Lstat/ModeSymlink arm in classifyCandidate, and the three symlink test files are all present there. The spec row states plainly that docs/planning/open-questions.md still shows OQ-28 unstruck on THIS branch -- that close-out belongs to PR #36 and arrives when it merges -- so the two files do not silently disagree. The status token is edited, never the row: check_post_audit_blockers requires the OQ-28 row to keep existing, and a closed row is the record.
konih
added a commit
that referenced
this pull request
Aug 9, 2026
…t lane #36) into the AUD-S13 lane # Conflicts: # CHANGELOG.md # docs/decisions/decisions.md
konih
added a commit
that referenced
this pull request
Aug 9, 2026
The provider containment lane (#36) merged while this lane was in review, so the denominator moved a second time: 4994 -> 5023. Re-measured at the new merge base and at the lane head -- 90.4041% -> 91.0810%, i.e. 4541 -> 4575 of 5023 (+34 covered), denominator identical on both sides, which is still the tests-only proof. New dated instance: true minimum ceil(0.9095 * 5023) = 4569 (4569 -> 90.9616% -> prints 91.0 -> PASS; 4568 -> 90.9417% -> 90.9 -> FAIL), so the slack is again 6 statements. Second re-measurement in one lane, both caused by main moving within a day -- recorded in the row as the reason the FORMULA is the durable part and the pair of integers is not.
konih
added a commit
that referenced
this pull request
Aug 9, 2026
…he arm-docs lane
Only CHANGELOG.md conflicted; it is regenerated in the next commit. Both
append-only tables auto-merged with nothing lost:
decisions.md 129 rows on each parent -> 131 merged (D-129/D-130 from
#36 plus D-134/D-135 from this lane); no D-id missing from
either side.
open-questions.md no OQ-id missing from either side; OQ-28 (#36) and OQ-29
(this lane) both present.
Subject is `:wrench: chore(release):` on purpose -- cliff.toml skips exactly
that and `:memo: chore(release):`, and everything else falls through to the
`.*` catch-all and would render a merge subject into the changelog (D-125).
konih
added a commit
that referenced
this pull request
Aug 9, 2026
konih
added a commit
that referenced
this pull request
Aug 9, 2026
…s on main The provider containment lane merged as 7513d79 while this lane was in review, so the caveat this branch carried -- that open-questions.md still showed OQ-28 unstruck here and the close-out would arrive with PR #36 -- is now false. It is removed from the spec's blocker row, from D-132 and from the gate's three prose copies; the OQ-28 row itself stays CLOSED and stays in the table, which is what check_post_audit_blockers requires. Swept by identifier (OQ-28, PR #36, 1e71cec) rather than by the line numbers fixed earlier, since the merge moved them.
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
1 participant
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.
Closes the P1 from OQ-28 (raised by PR #35 / AUD-S13, which is where OQ-28 itself lands — it is not on
mainyet, so this branch does not strike it through; the strike-through edit belongs on the rebase once #35 merges).The defect, reproduced
builtin/repo-fileheld path containment only —cleanRel/cleanRoots/underAnyRootare pure string guards.os.DirFSis documented in Go as not a security boundary, andisRegular'sfs.Statfollows symlinks. Path-level and filesystem-level containment are different guarantees and only the former was held, so the stated invariant "never a fact from outside the declared roots" was false in production:cmd/assent/provider_host.gocheckoutFSbuiltos.DirFS(<checkout>/head)— the merge-request head tree, i.e. content authored by the contributor under evaluation — and handed it to the builtin.Both forms reproduced RED before the fix (
internal/provider/builtin/repo_file_symlink_test.go,cmd/assent/run_provider_symlink_test.go):The last one is the production route:
assent run --checkout <dir>with abuiltin/repo-fileprovider, an MR whose head tree contains the symlink, and an APPROVE for a change no in-repo quota allows.What an attacker could read — before and after
Before: any absolute path on the host running assent, reachable from the MR under evaluation. Narrowed by two operator-controlled constraints only: the target must parse as a YAML/JSON mapping, and must carry a top-level key equal to a declared output name. The value then reaches the decision engine, and via
render.displayFactValue/formatMessageScalar(internal/render/message.go:72-85) a rulemessageinterpolating a non-sensitivefact echoes it into the MR comment (sensitive: truerenders[redacted]). Reachable forbuiltin/repo-fileandbuiltin/resource-owner, which share the FS.After — precisely: no fact can come from outside the checkout root, and none can be read through a symlink even inside it. Legitimate in-root files resolve exactly as before. This does not yet cover
cmd/assent/checkout.go, which reads the same tree with rawos.ReadFilefor the document under judgment — see the residual in the resource-owner section. That path is still live and is demonstrated by this PR's own fixture:symlinkBothSidesplants the link inbase/andhead/,collectTreefollows it, and the identical off-tree content is why the poisoned path stays out of the changed-file set.Fix layering — what each layer guarantees
checkoutFS→builtin.OpenRepoRoot(os.OpenRoot+(*os.Root).FS())classifyCandidateLstats every path component, refuses on any symlinkrootsclip, which layer 1 cannot see (topics/prod/quota.yaml -> ../../secrets/quota.yamlnever leaves the FS root yet leaves the roots), and catches the directory-symlink form under a bareos.DirFS, where an Lstat of the leaf alone reports an ordinary regular file.RepoFileOpts.FSdoc: "must be a symlink-safe root"os.DirFSunknowingly. Layer 2 is defence in depth, not a substitute — it can only observe what the injected FS reports.Every containment case in the new table runs against both flavours (
dirfsandrootfs) and must reach the same verdict — a case that only passed underrootfswould be testingos.Root, not this package.Two related edges: the
*os.Rootis closed byresolveRunFacts(every builtin read happens in-loop; nothing captures the FS beyond it), and a--checkoutthat cannot be opened as a containment root is now a hard error rather than a silent degrade to "no facts".go 1.25.0ingo.mod—os.OpenRoot,(*os.Root).FS(),io/fs.ReadLinkFSandfs.Lstatare all available;fs.Lstatfalls back tofs.Statfor a filesystem that cannot report links, which is exactly why layer 1 exists.Refusal semantics (decided, not defaulted)
unavailable, notinvalid— same class and fail direction as the existing"anchor outside declared roots". The operator's config is well-formed; the repo content is hostile.invalidin this file means malformed input. Both are non-resolved and both fail closed; consistency broke the tie.The refusal stops the walk-up; it does not skip to the next candidate. Skipping would silently fall back to a less-specific legitimate file and hide the escape behind an ordinary-looking resolution. The production test pins this: a legitimate
topics/quota.yaml(24) sits on the walk-up path and20 <= 24, so a skip-and-continue "fix" would still APPROVE.The reason string names the refused candidate and is contributor-readable — never the raw OS text (
statat …: path escapes from parent), which would leak a Go internal into an MR comment (ADR-0012).In-root symlinks: refused
Deliberate. A link that stays inside the FS root can still leave the declared roots, and
os.Rootcannot defend that because it does not know aboutRoots. So the only rule this layer can state safely is "no symlinks on the candidate path" — one rule, no per-link reachability reasoning. Refusing is the safer half of the trade-off; the friendlier half (follow in-root links) would hand the roots clip back to the attacker. Verified that no fixture, example or corpus file in the tree is a symlink (find . -type lis empty), so nothing legitimate regresses.builtin/resource-owner— assessmentIt shares the exposure, and worse: it reads the same FS with no roots at all, and the registry it loads decides who may approve.
Symlink (D-129, this PR): reproduced —
LoadResourceOwnerMapthrough a symlinked registry path loadedowner="attacker"from an off-tree file. Closed at layer 1 by the shared root FS, plus an explicit refusal insideLoadResourceOwnerMapso the failure is an error (no client ⇒ the owner fact never resolves) with a readable message instead of a rawpath escapes from parent.Precedence (D-130, this PR — a different vulnerability found in the same assessment):
loadResourceOwnerRegistrytriedrepoFSbeforeclient.FileAtRef(targetRef). Under--checkout,repoFSis the MR's own head tree, so an MR could shipgovernance/owners.yamlnaming its author as owner of the resource it is changing — a GUIDELINES safety-invariant-3 / ADR-0015 §1 trust-boundary violation with the same reachability class as the P1. Order is now target ref → (on error) checkout → error. Nothing depended on the old precedence: the live resource-owner path had no cmd-level test at all before this PR.Other consumers of that FS, enumerated:
builtin/repo-file(RepoFileOpts.FS)builtin/resource-owner(LoadResourceOwnerMapvialoadResourceOwnerRegistry)builtin/forge-groupscmd/assent/checkout.go—readIfPresent,collectTreecmd/assent/checkout.goreads the same head tree with rawos.ReadFile, and its own#noseccomment already states the assumption ("symlink TOCTOU is out of scope … fixing it belongs to the checkout-provisioning story"). Precision for whoever picks it up:filepath.WalkDirdoes not follow directory symlinks, soChangedFiles/collectTreecannot enumerate through a dir symlink — the exposure is a final-component file symlink only, which makes host file content become the "new" document under judgment. ConvertingdirCheckoutto a Root-based reader touches E1-S08 fold semantics and would jeopardise this fix, so it is reported rather than bundled. Operator: this is the residual after this PR.Mutation evidence
Every claim below was observed, not reasoned:
CONTAINMENT ESCAPE/ APPROVE outputs quoted at the top. The builtin-level cases fail underdirfswithout layer 1 involved, so layer 2 is independently proven.repoFS-first order inloadResourceOwnerRegistryturnsTestResourceOwnerRegistryLoadsFromTargetRefred withowner = "attacker"; probe reverted, verified absent from the tree.control_real_file_still_resolves(both FS flavours),TestRunCheckoutLegitimateQuotaStillResolves(same production route, still APPROVE),TestResourceOwnerRegistryRealFileStillLoads,TestResourceOwnerRegistryFallsBackToCheckout.git diffasserted after every mutation — 8 files in commit 1, 4 in commit 2; no phantom edits.checkoutFSerror (repoFS = nil) turnsTestResolveRunFactsFailsLoudlyOnUnopenableCheckoutred withfacts=map[] resolvedAt=map[]; probe reverted and verified absent.repo_file_test.go(16-case containment table + 8-case expiry table + undecodable table) was copied into this worktree and run against this fix — first at the containment commit (29 subtests green), and again on the final tree: 76 subtests pass, 0 fail, 0 skip with both files present — then restored. Note for reviewers: those tables live on AUD-S13: test-depth bundle (TEST-02/05/06) + coverage floor to 91% #35, not on this branch's base, so nothing was deleted here.requireSymlinksprobes by attempting a realos.Symlinkandt.Skipfs with the OS and the error. Noruntime.GOOSstring test, no silent skip.Gates
task checktask determinismtask dogfood-examplesbash hack/compare/exitgate_test.shgo mod tidy/go vet -tags e2e ./...All five re-run after merging
origin/main(e54a243, the AUD-S15 forge-port lift) into this lane; the only merge conflict wasCHANGELOG.md, resolved by regenerating per D-125's working rule.Records
checkout.goresidual.openspec/specs/p5-e5-provider-host/spec.md. No ADR needed — D-129 is an implementation-level security invariant under ADR-0004/0017, and D-130 is an application of ADR-0015 §1 rather than a new architectural decision.Risks
internal/provider/builtin/repo_file_test.go): avoided by putting every new case in new files (repo_file_symlink_test.go,resource_owner_symlink_test.go). AUD-S13: test-depth bundle (TEST-02/05/06) + coverage floor to 91% #35 also raises the coverage floor to 91%; this branch is at 90.3% against today's floor of 90 and adds ~0.1% — after both land, AUD-S13: test-depth bundle (TEST-02/05/06) + coverage floor to 91% #35's own tests must carry the rest.--checkoutpointing at a directory that cannot be opened as a root is now a hard error out ofresolveRunFacts(pinned byTestResolveRunFactsFailsLoudlyOnUnopenableCheckout). No test or fixture exercised that path before; the previous behaviour was "all facts unavailable", so the new one is at least as fail-closed and considerably louder.docs/planning/open-questions.mdedit must land on the rebase after AUD-S13: test-depth bundle (TEST-02/05/06) + coverage floor to 91% #35 merges.unavailablewith a reason naming the file — a loud, diagnosable break, not a silent wrong answer.