Found while planning #229. Out of scope there — that issue is a refactor of the drift walk, and this is upstream of it, in how reconcile constructs its plan.
This is not a secret leak. It is the inverse: the destination gets the template text rather than the resolved value. Nothing sensitive is written that was not already in the canonical source. The damage is a broken destination config, not a disclosure.
What happens
reconcile builds its plan from the templated canonical:
// internal/cli/reconcile.go:156
plan, err := render.Plan(secrets.ForRender(c), reg, agents, sc, projectRoot, s, userHome)
secrets.ForRender is a bare wrapper — it performs no substitution:
// internal/secrets/resolved.go
func ForRender(c source.Canonical) Resolved { return Resolved{c: c} }
So every op.Content in that plan still carries ${env:MY_TOKEN} / ${secret:github.token} verbatim. Choosing [o]verride on a drifted item then hands those ops straight to the adapter's Apply:
// internal/cli/reconcile.go:437-440
rw := render.NewWriter(s, home, userHome, sc, projectRoot, name)
if err := a.Apply(ops, rw); err != nil { … }
The destination — e.g. a live ~/.claude.json — receives the literal ${env:MY_TOKEN}. A subsequent agentsync apply rewrites it correctly, so the corruption is transient in the happy path, but until then the agent reads an unresolvable placeholder as its credential, and RecordOpsState records state for the templated content.
Why the other surfaces do not have this
apply plans from the resolved model:
| surface |
plan input |
apply (apply.go:143, :212) |
resolved — secrets.SubstituteCanonical |
status (status.go:183-188) |
ForRender, then SubstituteCanonical |
explain (explain.go:239-241) |
ForRender, then SubstituteCanonical |
diff (diff.go:102) |
ForRender only — but diff never writes, and it has a fail-closed UnresolvedSecretRefs gate |
reconcile (reconcile.go:156) |
ForRender only — and it writes |
reconcile is the only surface that both plans from templated content and writes to a destination. Its canMask check at :130 computes UnresolvedSecretRefs but uses it for masking display output, not for gating the write.
Why it is not a five-line fix
Switching :156 to SubstituteCanonical makes the override write correct values, but it also puts resolved cleartext into the items reconcile displays and diffs — which is exactly what the masking path at :130 exists to prevent, and that path is currently allowed to degrade (canMask == false) rather than fail closed. Getting this right means deciding:
- whether the override write and the display/diff should plan from different models (resolved for the write, templated for display), and
- whether
reconcile should refuse outright when UnresolvedSecretRefs is non-empty and an override is queued, the way diff refuses.
Both interact with the secret-handling invariants in CLAUDE.md and deserve their own review rather than a drive-by change inside a refactor.
Suggested test
A reconcile --auto-override run against a canonical carrying a ${env:…} reference, asserting the destination ends up with the resolved value and that state records the same. There is no test covering the override write path's content today.
Found while planning #229. Out of scope there — that issue is a refactor of the drift walk, and this is upstream of it, in how
reconcileconstructs its plan.This is not a secret leak. It is the inverse: the destination gets the template text rather than the resolved value. Nothing sensitive is written that was not already in the canonical source. The damage is a broken destination config, not a disclosure.
What happens
reconcilebuilds its plan from the templated canonical:secrets.ForRenderis a bare wrapper — it performs no substitution:So every
op.Contentin that plan still carries${env:MY_TOKEN}/${secret:github.token}verbatim. Choosing[o]verrideon a drifted item then hands those ops straight to the adapter'sApply:The destination — e.g. a live
~/.claude.json— receives the literal${env:MY_TOKEN}. A subsequentagentsync applyrewrites it correctly, so the corruption is transient in the happy path, but until then the agent reads an unresolvable placeholder as its credential, andRecordOpsStaterecords state for the templated content.Why the other surfaces do not have this
applyplans from the resolved model:apply(apply.go:143,:212)resolved—secrets.SubstituteCanonicalstatus(status.go:183-188)ForRender, thenSubstituteCanonicalexplain(explain.go:239-241)ForRender, thenSubstituteCanonicaldiff(diff.go:102)ForRenderonly — but diff never writes, and it has a fail-closedUnresolvedSecretRefsgatereconcile(reconcile.go:156)ForRenderonly — and it writesreconcileis the only surface that both plans from templated content and writes to a destination. ItscanMaskcheck at:130computesUnresolvedSecretRefsbut uses it for masking display output, not for gating the write.Why it is not a five-line fix
Switching
:156toSubstituteCanonicalmakes the override write correct values, but it also puts resolved cleartext into the itemsreconciledisplays and diffs — which is exactly what the masking path at:130exists to prevent, and that path is currently allowed to degrade (canMask == false) rather than fail closed. Getting this right means deciding:reconcileshould refuse outright whenUnresolvedSecretRefsis non-empty and an override is queued, the waydiffrefuses.Both interact with the secret-handling invariants in
CLAUDE.mdand deserve their own review rather than a drive-by change inside a refactor.Suggested test
A
reconcile --auto-overriderun against a canonical carrying a${env:…}reference, asserting the destination ends up with the resolved value and that state records the same. There is no test covering the override write path's content today.