Two canonicalisers that disagree on the #181 case: unify canonicalRoot onto paths.Canonical

[golimpio]

Summary

There are now two functions in the tree that answer "what is the canonical spelling of this path?", they are documented as if each were the only one, and they disagree on the exact case that caused #181.

	paths.Canonical (internal/paths/canonical.go)	canonicalRoot / canonicalPathForBoundary (internal/tools/boundary.go)
Relative path	Cleaned and returned unchanged, no filesystem access	Anchored to the process cwd via filepath.Abs
.. handling	Handed to EvalSymlinks uncleaned, so .. resolves in kernel order	filepath.Abs Cleans first, collapsing .. lexically
Missing path	Nearest existing ancestor resolved, tail re-joined	Same
Failure	Degrades to filepath.Clean	Same

Both were arrived at deliberately and both are currently correct for their own caller. The problem is that neither says it is one of two, so the next person to need canonicalisation will pick whichever they find first.

Why the divergences matter

Relative paths. paths.Canonical refuses to anchor them, because the daemon is a singleton whose working directory belongs to whichever client happened to spawn it — the silent cross-repository write of #181/#182. canonicalRoot anchors them with filepath.Abs. That is safe where it is used today (PathPolicy compares already-absolute roots), but it is exactly the wrong default to inherit by copy-paste.

Lexical ... canonicalPathForBoundary calls filepath.Abs before EvalSymlinks, and Abs Cleans — so a .. following a symlink is collapsed lexically, which diverges from the kernel's left-to-right resolution. This is the divergence #264 is landing a refusal for. paths.Canonical avoids it by passing the uncleaned path to EvalSymlinks, which resolves components in kernel order.

Suggested shape

Once #264 has landed:

1. canonicalRoot delegates to paths.Canonical, so there is one definition of "same place".
2. The unresolved-.. refusal stays where fix(tools): refuse an absolute path whose ".." the kernel resolves differently #264 puts it, in PathPolicy.Check, above the canonicaliser. That separation is the right one and must not be undone by the graft: Canonical is an identity function, not an authorisation check.
3. Decide explicitly what the absolute-anchoring behaviour should be at the boundary seam, rather than inheriting it. If PathPolicy genuinely needs a relative path anchored, it should do that itself, visibly, before calling the shared canonicaliser.
4. Add a rule to internal/arch.PrimitiveRules pinning the shared canonicaliser, so it cannot silently re-fork. PrimitiveRules exists for precisely this discipline and currently has no entry covering it.

Provenance

Split out of #270 (the #263 fix), which introduced paths.Canonical and deliberately did not touch internal/tools/boundary.go because #264 was in flight in that file and lands a property the graft must not undercut. Raised as a NIT by an independent review of #270, with the PrimitiveRules observation.

Not urgent and not a live defect — a trap for the next change, which is the cheapest kind of thing to fix before it fires.

[atlas-from-plumb]

Working this now. One correction to the issue as filed, which is the main reason for this comment: the sweep found five canonicalisers, not two.

Declaration	Verdict
internal/paths.Canonical	the Home — one definition of "same place"
internal/tools.canonicalRoot	the second one this issue names. Now an adapter: decodes file://, delegates the rest
internal/fsguard.canonical	Abs + EvalSymlinks. Keeps its anchoring, delegates the resolution — see below
internal/cli.canonicalXcodeRoot	a genuine fork nobody had noticed: Abs + Clean, no symlink resolution at all
internal/config.canonicalTaskHash / canonicalPolicyHash, internal/mcp.canonicalFor	not path canonicalisation — struct hashing and argument aliases

The Xcode one is the finding worth recording. Its result is the map key for the "one build-server flow per root" singleflight, so two spellings of one project — a checkout under a symlinked parent, or /tmp vs /private/tmp on macOS — each started their own concurrent xcodebuild against the same directory. Measured, not inferred: four runner calls where one flow makes two.

That is the argument for item 4 (the PrimitiveRules entry) being the load-bearing part of this issue rather than a tidy-up. Manual consolidation found two; the rule found five, and it found them by refusing to let anything be allowlisted without a stated reason.

Two decisions taken, since the issue asked for them explicitly

Item 3, absolute-anchoring at the boundary seam. PathPolicy does not need it, and no longer has it. A relative path now matches no root and is refused, and a relative root is dropped at construction. This is a user-visible behaviour change: a relative [workspace] extra_roots entry previously granted access relative to the daemon's working directory — a location neither the user nor the config author named, and config is a file a hostile clone can carry. Absolute roots are unaffected.

Anchoring is not always wrong, which is the part the issue's framing missed. internal/fsguard genuinely needs it, and in the opposite direction from the boundary policy. RefuseWalk matches macOS-protected directories by exact path, so an unanchored "." matches none and the walk is allowed — precisely when the working directory is $HOME. There, refusing to anchor fails open. So the rule pins the resolution to one implementation and leaves anchoring as a separate, visible decision each caller makes above it.

Item 2, the .. refusal, is untouched. It stays in PathPolicy.Check, above the canonicaliser, exactly as #264 landed it.

PRs

• fix(tools,fsguard): one canonicaliser, not two that disagree #284 — items 1–3: the fold, the behaviour decisions, tests. Ready.
• arch: pin path canonicalisation so it cannot re-fork (blocked on #284, #282) #285 — item 4: the PrimitiveRule. Draft, blocked on fix(tools,fsguard): one canonicaliser, not two that disagree #284 and fix(config): bind the command allow-list, shell gate and Xcode server to approved content #282.
• fix(config): bind the command allow-list, shell gate and Xcode server to approved content #282 (a peer's) carries the canonicalXcodeRoot fold, because that PR is rewriting those files and a rebase onto a changed signature was the riskier option.

#285 is deliberately held back rather than merged green. Merging it first would have passed CI while shipping an allowlist entry whose stated reason is false for the code on main — the precise failure primitives.go's own header warns about, and one no test can catch. For that rule the tests passing is not the check; the reason text being true is.

[atlas-from-plumb]

Done — all four items are on main. Closing.

Item	Where	State
1. canonicalRoot delegates to paths.Canonical	#284 (ed15f67a)	merged; canonicalPathForBoundary deleted
2. The .. refusal stays above the canonicaliser	#284	unchanged in PathPolicy.Check; #264's and #270's tests pass unedited
3. Decide the absolute-anchoring behaviour explicitly	#284	decided — see below
4. PrimitiveRules entry so it cannot re-fork	#285 (c478bf9e)	merged

Verified on merged main by grepping the merged files, not by trusting the merge results.

The sweep found five canonicalisers, not two

internal/cli.canonicalXcodeRoot was the one nobody had noticed: Abs+Clean with no symlink resolution, and its result is the map key for the "one build-server flow per root" singleflight — so two spellings of one project each started their own concurrent xcodebuild. Measured: four runner calls where one flow makes two. Folded in #282 by a peer, since that PR was rewriting those files.

Item 3, decided

PathPolicy does not need absolute-anchoring and no longer has it. But the issue's framing — that anchoring is the hazard — is right for the boundary policy and backwards for internal/fsguard: RefuseWalk matches macOS-protected directories by exact path, so an unanchored "." matches none and the walk is allowed, precisely when the working directory is $HOME. Refusing to anchor there fails open. So the rule pins the resolution to one implementation and leaves anchoring a separate, visible decision each caller makes above it — which is what the two allowlist entries now record.

That decision had a consequence worth naming: a relative root could then brick a session, because detection resolved it against the daemon's cwd while the policy declined to. Independent review caught it, a first fix moved the failure rather than removing it, and the invariant now lives at buildPathPolicy where all six pin routes converge. Full history on #284.

What this did not close

#290 — lockPathKey is a sixth canonicaliser, and a live defect rather than a naming problem: it has no missing-ancestor walk, so two spellings of one not-yet-created file take different write-lock mutexes. It also anchors relative paths, which canonicalRoot now refuses to do. The name-based rule cannot see it, which is the structural point — #290 also carries the case for a CallRule on filepath.EvalSymlinks, the mechanism that would have caught it and the eight other inline resolution sites.

So: this issue is closed, and the thing it was really about — that a shared primitive re-forks unless something enforces it — is now a test, with one known outstanding instance tracked separately.