Skip to content

reconcile/diff cannot mask a destination secret whose vault/env value has since rotated #254

Description

@spxrogers

What happens

agentsync diff and the interactive reconcile prompt (and its [d]iff) mask secret values in the displayed source/destination text with a value-keyed redaction map: secrets.CollectResolved resolves every ${secret:…} / ${env:…} reference in the canonical model now and maps each resolved value back to its placeholder; secrets.MaskResolved then does strings.ReplaceAll(text, value, placeholder) for each value (internal/secrets/mask.go:22, :133; call sites internal/cli/diff.go:131,346-347 and internal/cli/reconcile.go:357,1024-1025).

That map can only mask values it holds. If the vault entry or environment variable has been rotated since the last apply, the destination file still carries the old cleartext, the map holds only the new value, every reference resolves (so canMask is true and the display does not fall back to SHA prefixes), and the old credential is printed verbatim in the diff.

Reproduction (observed while building the #232 byte-identity fixture)

  1. mcp/gh.toml has env = { GH_TOKEN = "${env:GH_TOKEN}" }.
  2. GH_TOKEN=ghp_OLD agentsync apply~/.claude.json carries ghp_OLD.
  3. Edit the destination so the item drifts, then run GH_TOKEN=ghp_NEW agentsync reconcile (or diff).

The source side shows ${env:GH_TOKEN}; the destination side (and the character-level diff fragments) shows ghp_OLD in cleartext. In the fixture this surfaced because the session's real environment set GH_TOKEN to a different value than the harness's fake one; the harness now pins the variable for the whole run, so the fixture no longer exercises it.

Why it is not a leak in the apply sense, but still a display bug

Nothing is written anywhere: this is the display path only, and the value it shows is one that is already sitting in the user's own native config. But the whole point of canMask (fall back to SHA prefixes when a reference cannot be resolved) is "never print a value we can't mask", and a rotated value is exactly such a value that the current check does not detect.

Possible directions (not decided)

  • Treat a secret-bearing field whose destination text differs from the source's resolved value as unmaskable for display and fall back to the SHA-prefix rendering for that item (same policy as !canMask).
  • Or mask by position: the templated source tells us which fields carry references; redact the destination's value at the same JSON pointer regardless of what it is.
  • Or keep value-keyed masking but also include the value recorded at the last apply (the state file hashes content but does not store the value, so this would need new state and is probably the wrong trade).

Out of scope for #232 (behaviour-preserving refactor; observed in both the base and refactored binaries identically).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions