[Feature]: enrich audit entries with reason, argument digest, and principal #3668

Activity

1. github-actions commented on Aug 10, 2026

   

   github-actionsbot

   on Aug 10, 2026 – with GitHub Actions

   More actions

   🟡 Contributor Check: MEDIUM

   Check	Result
   Profile	MEDIUM
   Credential	MEDIUM
   Overall	MEDIUM

   Automated check by AGT Contributor Check.
2. github-actions

   added

   needs-review:MEDIUMContributor check flagged MEDIUM riskContributor check flagged MEDIUM risk

   on Aug 10, 2026
3. amriksingh0786 commented on Aug 18, 2026

   

   Amrik Singh Khalsa (amriksingh0786)

   on Aug 18, 2026

   Contributor

   More actions

   I'd like to pick this up. Posting what I found and a few questions first, since the coordination note asks for that.

   What's in the code today

   Everything that writes to the audit log goes through one function, recordAudit at lib/policy.mjs:612, called from five places (:163, :241, :1194, :1251, :1296). That makes this easier than I expected in one way and harder in another.

   The easy part is reason. It's basically already there: all five call sites have decision.reason in hand, and recordAudit simply doesn't pass it along.

   principal and the argument digest are genuinely new work though. Neither the tool arguments nor any notion of identity reaches recordAudit at the moment.

   The bit I'd like to agree on before writing any code

   Adding fields to the hashed payload turns out to be a chain migration rather than a field addition. appendAuditEntry throws if the existing log fails verification (lib/audit.mjs:14), and the plugin fails closed on audit errors, so a naive change means the first write after an upgrade denies everything.

   I wanted to be sure about that rather than assume it, so I wrote a two-entry log with the current code and rehashed it with reason added to the payload:

   v1 log verifies today:        true
   same entries under new hash:  false
   next write THROWS:            Audit log failed hash-chain verification.
   

   What I'd suggest instead is versioning the entries:

   • entries with no v field stay v1, and verify against the current five-field payload and serializer, byte for byte
   • new entries carry v: 2 and the extended payload
   • previousHash still links across the boundary, so a single file can hold a v1 prefix and a v2 suffix and still verify end to end
   • nothing gets migrated, and v1 entries age out through normal rollover

   I'd also move v2 to a sorted-key serializer. Right now computeHash digests JSON.stringify(payload), so the hash depends on property insertion order, which feels risky for a format whose main value is that someone outside the toolkit can verify it. v1's serializer would stay exactly as it is.

   Questions

   1. How much should the digest hide? Tool arguments are often low-entropy (a path, a URL, a branch name), so a plain SHA-256 gives you joinability but not secrecy, and a short argument is recoverable. Is it better to document that boundary clearly, or to support an optional HMAC key? My instinct is to do both: document the limitation, and allow a configured key for people who need more.

   2. Which arguments count as "relevant"? Including all of them is predictable and easy for a verifier to reproduce. A per-tool allowlist avoids digesting large blobs. I lean towards all of them with a size cap, but I'm happy to go the other way if you'd rather.

   3. What shape should principal be? An opaque subject string, or something structured like { sub, iss }? Structured mirrors OIDC and holds up better when more than one identity provider is involved. Either way I'd only take it from explicit config or an AGT_OPENCODE_PRINCIPAL env var, never from tool arguments or model output, since otherwise it stops being evidence.

   4. How does this sit with fix(opencode): preserve audit chain verification across rollover eviction #3250? That PR is open against the same file and changes eviction so the chain stays verifiable, which is the same verifyAuditEntries path I'd be versioning. Would you rather this went on top of it, or waited for it to land?

   Happy to change any of this. If the approach looks reasonable I'll open a PR with Closes #3668, doing the versioning and canonicalization first with tests, then adding the four fields one at a time.
4. amriksingh0786 commented on Sep 27, 2026

   

   Amrik Singh Khalsa (amriksingh0786)

   on Sep 27, 2026

   Contributor

   More actions

   Following up on the questions above. They have been open for a while, so rather than let this sit I have implemented it with defaults and opened a PR. Everything below is easy to change if you would rather go another way.

   • Canonicalization: keys sorted by UTF-16 code unit, the RFC 8785 ordering, so someone not running AGT can reproduce a hash. The current JSON.stringify hash depends on property insertion order.
   • argsDigest: SHA-256 by default, documented as joinable rather than secret, with AGT_OPENCODE_AUDIT_HMAC_KEY switching it to HMAC-SHA256. A key under 32 bytes is refused rather than used.
   • Arguments digested: all of them, with a 1 MiB cap and an argsTruncated flag, rather than a per-tool allowlist.
   • principal: structured { sub, iss? } after OIDC, sourced only from operator config or AGT_OPENCODE_PRINCIPAL_SUB / _ISS, never from tool arguments or model output.
   • v1 entries: left to age out through normal rollover, no migration.

   On sequencing: #3250 closed unmerged on 2026-08-29, so nothing is in flight on lib/audit.mjs and this no longer has to wait on it. The rollover bug that PR was fixing is still there and I have kept it out of scope, since it is independent of versioning. Happy to port the claude-code seam fix separately if that is useful.

   The one thing I would flag for review is that widening the hashed payload is a chain migration rather than a field addition: appendAuditEntry throws on a failed chain and the plugin fails closed, so a naive change would deny every request on the first write after an upgrade. The PR describes how versioning avoids that.
5. MohammadHaroonAbuomar

   closed this as completedin #4164on Sep 28, 2026