Skip to content

feat: every secret read leaves an audit line, visible in Secrets (goal 0203 S3) - #432

Merged
alicoding merged 2 commits into
mainfrom
goal-0203-s3-secret-read-audit
Aug 25, 2026
Merged

feat: every secret read leaves an audit line, visible in Secrets (goal 0203 S3)#432
alicoding merged 2 commits into
mainfrom
goal-0203-s3-secret-read-audit

Conversation

@alicoding

@alicoding alicoding commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Summary

Every resolution of a vault entry now writes one audit line — S3's own contract closed. A sibling of the MCP call audit trail's store shape, never its table: internal/adapters/secretaudit (pure types) + internal/adapters/secretauditstore (its own secret_access table in the execution SQLite file) record entry id + denormalized label (a rename never rewrites history), which seam resolved it, the run/workflow id when in-run, and outcome.

Seams instrumented, with context

Context Seam Real-run attribution
mcp-server-spawn configuremcpserver.go's resolveMCPServer (composition's SetMCPServerLookup) RunID/WorkflowID from ExecContext when a workflow triggers it; empty for RefExists' dangling-ref check
exec-env configureexecenv.go's resolveExecEnv same
http-header configureservice_requestauth.go's resolveHTTPRequest same
configure-tools-preview ListMCPServerTools (via a new resolveMCPServerWithAccess split) always empty — S2's own found gap: this spawns a real MCP server outside any run, previously invisible. Closed here.
ui-reveal SecretService.RevealSecret n/a
ui-copy SecretService.CopySecretToClipboard n/a

composition.SecretAccessRun (RunID/WorkflowID) threads through the three composition-level lookup seams (lookupMCPServerFn/lookupExecEnvFn/lookupHTTPRequestFn), mirroring the existing ExecContext.WorkflowID/currentRunID(ctx.RunContext) pattern already established for atlas-card nodes — no new plumbing invented. Every non-run caller (RefExists, static graph validation) passes the zero value; never an error.

Deliberate opt-out

RedactKnownSecrets stays unaudited (comment at the call site) — it enumerates the vault only to scrub output on a failure path, never exposing a value; auditing it would bury real reads in noise.

Retention

AuditRetentionKeep = 10000, pruned at OpenAudit boot — same number and "prune at boot" posture as mcpauditsvc.RetentionKeep.

Surface

Secrets view gains Access history: a global newest-first list from the header (clock icon), and a per-entry filtered view from the detail dialog's own footer button — one component, SecretsAccessHistoryDialog, driven by a new SecretService.ListSecretAccess RPC. Copy in user vocabulary ("Read by workflow …", "Shown to you", "Copied to the clipboard", "Couldn't be read"), relative time via the existing shared formatUpdated helper, workflow names resolved from CompositionService.Workflows() (never a raw id shown). Reviewed live against a running server-mode instance before opening this PR (both the per-entry and global dialogs render exactly the copy/layout described above — screenshots captured but not attached to this description).

Contract question I could not answer (reported, not decided)

The S3 contract's CONTEXT enum listed workflow-run alongside the six I built. I could not find any seam that would ever produce it distinctly from the six above — the three vaultref call sites already partition by entity kind (mcp-server-spawn/exec-env/http-header), and no fourth path calls the resolver. I implemented the six contexts the contract's own "instrument at the RESOLUTION seams the survey mapped" list actually names, and dropped workflow-run as an apparent enum-listing artifact rather than inventing a seam for it. Flagging rather than silently deciding it was load-bearing.

Two pre-existing findings, observed not fixed (out of scope for this slice)

  • internal/domain/composition/graph.go's validateOutputBindingSecrets calls the full lookupHTTPRequestFn (keychain secret + header vault-ref resolution) during static graph validation, just to read .OpenAPISpec — an existing over-fetch. With this change it also means a save/validate pass on a workflow using a vault-ref header creates an http-header audit row with no run/workflow id attached (honest signal, just surprising if unexpected). Narrow blast radius (only workflows with outputBindings + a vault-ref header), pre-existing, not touched here.
  • configureservice_requesttest.go's TestHTTPRequestOperation ("test this request" RPC) passes req.Headers straight through to the real HTTP call without resolving vault:<id> references at all — a header literally reading vault:abc123 would be sent as-is. Pre-existing gap, unrelated to auditing, not touched here.

Seeded proof

Checked: no TestSeeded* test executes a real MCP spawn or exec-env resolution that reaches the audit trail — the existing seeded "Example: uses a stored secret" workflow (secretguard_seed_test.go) is denied at the guardrail gate before the step (and its vault-ref resolution) ever runs, so it only proves S2's label-derivation path, not a real read. Per the brief's own fallback, coverage is at the service level instead: configureservice_secretaudit_test.go and secretservice_audit_test.go drive the real composed path (a real SecretService wired via the exact SetSecretResolver seam production uses) through every seam.

Test evidence

  • go test ./internal/... — all green (adapter tests for the new store: insert/list/filter/paging/retention/error-truncation; service tests: each seam → exactly one record with the right context; negative test: a plain non-vault: value never touches the resolver at all).
  • golangci-lint (default + --build-tags=server), gocognit-new, depguard-new — all clean (one legacy function's complexity paid down via extraction while touching it, not left over threshold).
  • go vet, tsc, eslint, vitest — all clean.
  • secrets.spec.ts (extended) — reveal + copy each produce a visible Access-history row, from both the per-entry and global dialogs; run against the real mill-server binary.

🤖 Generated with Claude Code

https://claude.ai/code/session_012im1JxQQV2ahnXzZDdVmZq

@alicoding
alicoding enabled auto-merge (squash) August 25, 2026 06:20
alicoding and others added 2 commits August 25, 2026 02:27
A sibling of the MCP call audit trail's own store shape, never its
table: internal/adapters/secretaudit (pure types) and
internal/adapters/secretauditstore (its own secret_access table in
the execution SQLite file) record who read a vault entry, when, and
why -- entry id + denormalized label, context, run/workflow ids when
in-run, outcome.

Instrumented seams: SecretService.ResolveSecretValue (the choke
point every vaultref resolver call site funnels through --
mcp-server-spawn, exec-env, http-header, each tagged by its own
caller), ListMCPServerTools' Configure-page preview path (S2's own
found gap, closed here: it spawns a real MCP server outside any
workflow run, previously invisible), and the UI's RevealSecret/
CopySecretToClipboard. RedactKnownSecrets stays deliberately
unaudited -- it enumerates the vault only to scrub output on a
failure path, never exposing a value.

composition.SecretAccessRun threads RunID/WorkflowID from a node's
own ExecContext through the three lookup seams (lookupMCPServerFn/
lookupExecEnvFn/lookupHTTPRequestFn) so an in-run read is
attributable to the run that triggered it; every non-run caller
(RefExists' dangling-reference check, static graph validation) passes
the zero value, which is never an error.

Two service-level test files prove the full composed path (a real
SecretService, a real ConfigureService, wired via the exact
SetSecretResolver seam production uses) drives each seam to exactly
one audit record with the right context.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012im1JxQQV2ahnXzZDdVmZq
SecretsAccessHistoryDialog is one component for both entry points:
the Secrets view header opens it with every read across the vault,
newest first; the detail dialog's own "Access history" footer button
opens it filtered to that one entry. Copy in user vocabulary ("Read
by workflow <name>", "Shown to you", "Copied to the clipboard"),
relative time via the existing shared formatUpdated helper, Primer
ActionList mirroring ActivityMCPCalls' own audit-log shape. Read-only,
server-paged against the new SecretService.ListSecretAccess RPC.

secrets.spec.ts extended: reveal + copy each produce a visible
Access-history row, from both the per-entry and global dialogs.

Incidental: regenerating bindings for the new RPC also picked up
already-merged, previously-uncommitted binding drift (guardrail.Rule's
BuiltIn/Seed fields, goal 0207's settingssvc doc updates) -- the three
frontend call sites constructing a bare Rule literal needed the two
new required fields to keep tsc green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012im1JxQQV2ahnXzZDdVmZq
@alicoding
alicoding force-pushed the goal-0203-s3-secret-read-audit branch from 922ab54 to e65259f Compare August 25, 2026 06:30
@alicoding
alicoding merged commit 92febd8 into main Aug 25, 2026
27 checks passed
@alicoding
alicoding deleted the goal-0203-s3-secret-read-audit branch August 25, 2026 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant