feat(audit): support RFC 8693 act delegation chains

[JAORMX]

Summary

Adds typed, additive RFC 8693 act delegation-chain support to the audit package, so audit events can capture the actor identity and full delegation chain of a delegated token — not just the terminal sub. This closes the shared-library side of stacklok/toolhive#6035 ("Alice did X" vs "Alice's agent did X on her behalf").

This package is intended to be canonical: toolhive's pkg/audit/pkg/auth delegation schema (stacklok/toolhive#6046, #6096) will be reconciled to match this one at migration time.

What changed

• DelegatedActor — one hop, identified by typed iss+sub only (OIDC Connect Core §5.7: the (iss, sub) pair is the only stable, unique actor identifier; RFC 8693 §6 data minimization). All other per-hop claims are retained in memory only and are never serialized or logged (PII guard; NIST AU-3(1)/(3) audit-record minimization); accessible via Extra() (shallow copy, documented as must-not-be-logged).
• DelegationChain — flattened, outermost-first (Chain[0] = current actor, per RFC 8693 §4.1 the only hop that may drive access control; prior hops informational). Current-actor-first keeps Chain[0] stable under truncation. Truncated/Omitted/Malformed are always emitted — an incomplete or non-conformant chain is never silently indistinguishable from a complete, well-formed one.
• Record-and-flag malformed handling — ParseDelegationChain(raw, maxDepth) *DelegationChain never fails. Non-conformant input (non-object act, non-object nested act, non-string iss/sub) sets Malformed plus a low-cardinality MalformedReason enum (act_not_object, nested_act_not_object, iss_not_string, sub_not_string) and preserves every hop parsed before the violation. An audit record must be producible for exactly the tokens that violate expectations: dropping the record on malformed attacker-influenceable input is an indicator-blocking primitive (MITRE ATT&CK T1562.006; CWE-223/CWE-778), and per RFC 8693 §4.1 prior actors are informational-only, so a malformed chain cannot legitimately fail authorization — recording it is its only purpose. Strict rejection belongs on the token-issuance path (see Auth server can mint an act claim that violates RFC 8693 §4.1 toolhive#6093).
• Chain is always allocated — the wire always carries "chain": [], never "chain": null (guarded by a test asserting emitted bytes).
• Named map types (e.g. jwt.MapClaims) are accepted at any nesting level via a reflection fallback, so programmatically built claims don't silently parse as non-objects.
• AuditEvent.DelegationChain (json:"delegation,omitempty") + WithDelegationChain builder. Zero chains (no hops, no truncation, no malformation) are cleared; hopless-but-malformed chains are kept. Subjects is untouched — the delegating end user is the token's top-level sub in Subjects, not a chain entry.
• LogTo emits the chain via its JSON marshaling (slog.Any), so the slog and JSON wire shapes of the same event are structurally identical — one parser serves both (guarded by a shape-equality test).
• Fixed the pre-existing doc.go bug and added a Delegation Chains section documenting ordering, the PII stance, and the malformed-input policy.

Backward compatibility

Purely additive — no breaking changes. One additive struct field with omitempty; no constructor/signature changes; Subjects contract unchanged. With no chain set, JSON and log output are byte-identical to before (guarded by tests). No new dependencies (stdlib only).

Design decisions

• Flag, not error. Evidence: NIST SP 800-53 AU-3(f) (records must identify subjects, no "only if parseable" qualifier) + AU-5 (alert, don't halt); OWASP ASVS 5.0 16.5.3/16.5.4 (fail closed on the operation, never lose the error details from the log); RFC 9413 (forbids silent tolerance — a loud flag satisfies it, discarding violates it); syslog RFC 3164 §4.3.3 record-and-flag precedent; CloudTrail's first-class userIdentity.type: Unknown. The reason field is an enum, not free text, per OTel error.type low-cardinality guidance, so it is safe as a metric/alert dimension.
• Depth default 16 (not 10): this is a parse-time defensive ceiling for a shared library, deliberately above any consumer's minting cap, so that for a consumer passing its mint cap (toolhive: 10; draft-mw-spice-actor-chain §21.9 RECOMMENDED default: 10), truncated: true is a genuine signal that a token came from outside its own issuance path.
• Chain type lives directly in the audit package, auth-agnostic — ParseDelegationChain takes an already-decoded any, no JWT dependency.
• No auto-mirroring of the actor into Subjects (avoids smuggled side effects in builders).

Test plan

task green (golangci-lint 0 issues, go vet clean, race tests pass); task license-check passes. Audit package coverage 99.0%. Tests assert emitted JSON keys, not Go struct fields (tag renames cannot pass with a green suite); pin "chain":[] vs null; verify slog/JSON shape equality; cover nested parse (0/1/N hops), outermost-first ordering, exactly-at-cap / cap+1 / depth-1000 truncation with exact omitted counts, malformed-past-cap semantics, non-string sub/iss (flag + in-memory retention), named map types, first-reason-wins, extra-claims PII guards in both JSON and slog output, JSON round-trip, and omit-empty/backward-compat guards.

Refs stacklok/toolhive#6035

[jhrozek]

Heads up — this overlaps with work that already merged on the toolhive side, and the two schemas currently disagree on every field name.

stacklok/toolhive#6046 (merged earlier today) implemented the same feature for pkg/audit, and stacklok/toolhive#6096 is a follow-up hardening it. Since audit/event.go here was graduated out of toolhive in #81, both PRs are adding a delegation field to what is logically the same struct — so whichever schema survives the eventual pkg/audit → core/audit migration silently rewrites audit log output for anyone keying on the other one.

Where we diverge:

	toolhive pkg/audit + pkg/auth	this PR
event field	delegationChain,omitempty	delegation,omitempty
hop list	Actors → actors	Chain → chain
dropped counter	dropped_count,omitempty	omitted (no omitempty)
per-hop iss	folded into act_claims	typed Issuer → iss
other per-hop claims	serialized as act_claims, minus internal tsid	never serialized (unexported extra, json:"-")
malformed act	tolerant: Malformed bool flag, partial chain kept	hard error: ErrNotJSONObject / ErrNestedActNotObject
default depth	10	16
slog shape	one slog.Any("delegation_chain", chain) blob	slog.Group with hops.hop_N.{iss,sub}

The good news is that the highest-risk thing agrees: actor ordering is identical — both outermost-first, both keep the outermost hops on truncation, both report not-truncated when the chain is exactly at the cap. That was the one divergence that would have made audit records actively misleading, and it is not one.

Two things worth deciding rather than discovering later.

1. The PII question is a real design disagreement, and I think you are right. We serialize arbitrary per-actor act members as act_claims and filter only tsid; this PR refuses to serialize them at all, citing RFC 8693 §6 data minimization. That is the stronger position. Worth noting the concrete reason it matters on our side: Identity.MarshalJSON redacts tsid from top-level claims but marshals the delegation chain verbatim to admission webhook receivers, so before the internalClaims filter in #6096 an act.tsid from a foreign IdP would have leaked there. Your design makes that class of bug impossible instead of merely filtered.

2. Malformed input: flag or error? You return ErrNotJSONObject; we set malformed: true and keep whatever hops parsed. Our reasoning was that an audit record should be able to say "the caller presented a token asserting delegation that we could not read", which is distinct from "no delegation" — and because signature and issuer validation run first, a malformed act can only come from a trusted issuer, so it flags an issuer bug worth recording rather than client noise. That argument is specific to the audit sink; for a library parser, returning an error is defensible. Either way the two should not disagree.

Concretely relevant to (2): stacklok/toolhive#6093 — the toolhive auth server can itself currently mint a non-conformant nested act, because the token-exchange handler copies a prior act without checking it is a JSON object, and its depth counter returns 0 for a non-map so the depth gate waves it through. Malformed chains are reachable from our own issuance path, not just hypothetically from third parties.

Findings from the #6046 review that may apply here too

Listing these so we do not have to find them twice.

• Chain has no omitempty, so a nil slice marshals to "chain": null. We hit exactly this: admitting an actorless chain made "actors": null reachable where it never had been, which breaks jq array iteration and Python for loops on precisely the malformed tokens the feature exists to surface. We now always allocate the slice. Your hard-error path may make it unreachable — worth confirming, and pinning either way, since assert.Empty passes for both null and [].
• Assert the emitted JSON keys, not just the Go struct fields. Ours were struct-field-only, so a tag rename would have broken every downstream consumer with a green suite.
• Check the exactly-at-cap and cap+1 boundaries explicitly, including the precise dropped/omitted count for a chain much deeper than the cap. Our accumulating DroppedCount += would have passed as a plain =, because no test supplied a non-zero incoming count.
• A non-string sub. We captured sub only when it type-asserted to string but dropped the key from the claims copy unconditionally, so {"sub": 123} produced a completely empty actor object — an anonymous actor with no trace that an identifier had been present. Your typed Subject/Issuer fields may have the same shape of hole.
• A named map type will not type-assert. jwt.MapClaims is map[string]interface{}, but a .(map[string]any) assertion fails on it. In practice only the top-level claims object is jwt.MapClaims — nested values come from encoding/json as plain maps — so this is fine on the real path, but a programmatically-built jwt.MapClaims{"act": jwt.MapClaims{...}} silently yields nothing.
• Cover every emitter, not a representative one. Our audit type repeats the attach call in eight methods and only two were tested; the six uncovered ones included every terminal failed/timed-out event, so dropping the call from any of them would have shipped silently.
• If both keep a depth default, reconcile 10 vs 16 or document the distinction — parse ceiling vs mint cap. The toolhive mint cap is 10, so a locally-issued chain never truncates, which makes truncated: true a genuine signal that a token came from elsewhere. That property is worth preserving deliberately rather than by accident.

Happy to change ours if you would rather core be canonical — nothing outside toolhive depends on our field names yet, so now is the cheapest moment. What I would like to avoid is both shipping and then reconciling after someone has written alerts against one of them.

[jhrozek]

Overlaps a merged toolhive PR, and the hard-error contract is doing damage

Two things, then four one-click suggestions that I applied and verified on this branch before proposing them.

1. Schema decision (cross-repo, not code). toolhive#6046 merged this same feature earlier today with a different wire schema:

	toolhive (merged)	this PR
event field	delegationChain	delegation
hop list	actors	chain
dropped counter	dropped_count,omitempty	omitted
per-hop iss	folded into act_claims	typed iss
other per-hop claims	serialized as act_claims	never serialized
malformed act	Malformed bool, partial chain	hard error
default depth	10	16
slog shape	one slog.Any blob	slog.Group, hops.hop_N

No log consumer can be written against both. Ordering and truncation direction already agree, so this is names plus one design call. Core's audit is toolhive's pkg/audit graduated (core#81), so core's keys win by construction and toolhive's emitted keys will change silently under existing consumers when that migration lands. Nothing outside toolhive depends on either set yet — cheapest moment to converge is now.

On the one substantive difference, your unexported extra should win. toolhive marshals the chain verbatim to admission-webhook receivers, so a foreign act.tsid reached third parties unredacted until #6096 added a filter. Yours makes that class of bug unrepresentable rather than filtered.

2. (nil, err) is the root of most of what I found. Per toolhive#6093, our own token-exchange handler copies a prior act without checking it is an object (actChainDepth returns 0 for a non-map, so the depth gate waves it through) — malformed chains are reachable from our own AS. Because signature and issuer validation run first, a malformed act can only arrive from a trusted issuer, so it flags an issuer bug, which is exactly what an audit trail exists to capture. Returning a nil chain means it vanishes from the record, and the record cannot distinguish "no delegated token" from "token asserted delegation we could not read".

Adopting toolhive's Malformed bool and never returning a nil chain closes six findings at once:

finding	before	after
chain vanishes on malformed act from a trusted issuer	(nil, err)	partial chain + Malformed: true
depth invariance broken — h1→h2→h3→act:"garbage" yields 2 hops / omitted=2 at maxDepth=2, but a hard error at maxDepth=16	record vs nothing, operator's knob decides	same record either way
omitted counted a bare string as a dropped hop	omitted=2 for one real dropped hop	objects only
json.Marshal(ParseDelegationChain(nil, 0)) → {"chain":null,…}, breaking jq iteration	null	[]
{"sub":"h1","act":null} errored, though 4.1 only makes present-but-not-an-object non-conformant	error	terminates the chain
{"sub":123,"iss":["a"]} → empty hop, Extra() nil, evidence gone	dropped	preserved in Extra()
dead branches at :129-133, :149-153, :178, :191-195 — collectively every uncovered statement in the file	unreachable code	gone

Test churn is two lines. In the table runner, assert.Nil(t, c) becomes:

require.NotNil(t, c, "a chain must be returned even on error")
assert.True(t, c.Malformed, "error path must set Malformed")

and the act present but not a map case's errContains becomes "got string". Worth adding cases for act: null, a malformed act past the cap, and {"sub": 123}.

One test gap independent of all the above: the json: keys are asserted nowhere. Mutation-tested on this branch — chain→actors and omitted→dropped_count,omitempty each keep go test ./audit/ green, and the second silently defeats the "truncation is never silent" promise in the DelegationChain doc comment. The slog keys are well pinned by TestAuditEventLogTo_Delegation; it is TestAuditEventJSON_RoundTrip that is tautological, since it marshals and unmarshals the same type. Suggested addition:

var wire map[string]any
require.NoError(t, json.Unmarshal(data, &wire))
d := wire["delegation"].(map[string]any)
_, isArray := d["chain"].([]any)
assert.True(t, isArray, `"chain" must be a JSON array, never null`)
assert.Contains(t, d, "omitted", "omitted must be present even when zero")

Also worth reconciling with §1: LogTo emits delegation.hops as an object keyed hop_0…hop_N, while json.Marshal emits delegation.chain as an array. Two shapes for one datum, and ordinal-keyed objects cannot be iterated or filtered in most log query languages.

Verification. I applied all four suggestions below plus the two test edits locally on 585e418: task lint clean, go test -race ./audit/ green, coverage 91.3% → 98.7% (100% on ParseDelegationChain, countRemainingHops, and extraClaims — the dead branches were the uncovered statements), and delegation.go gets 27 lines shorter. Field names are yours to settle per §1; I kept toolhive's Malformed spelling on the assumption that convergence is the goal.

Nothing here is blocking on my account once the schema question has an owner — but I would not merge a second incompatible schema without that answer.

[JAORMX]

Thanks for the thorough writeup — this was exactly the right moment to catch it. Direction first: let's make toolhive-core canonical and fix toolhive to match. The fact that #6046 already merged shouldn't weigh on what the shared library does; and since stacklok/toolhive#6082 means the token-exchange grant currently has no production traffic, the toolhive wire contract is still cheap to change.

Since your comment I did a deeper survey of the standards and production landscape (RFC 8693/9413, the transaction-tokens / identity-chaining / actor-chain drafts, NIST SP 800-53 AU-family, OWASP ASVS 5.0 V16, MITRE ATT&CK, CoSAI Agentic IAM v1.0, and the audit-log shapes of CloudTrail, GCP, Kubernetes, Entra, Envoy XFCC, Elasticsearch). The PR is updated (585e418 → 4aad6c9) with the result. Point by point:

1. Malformed input: you were right, and it's stronger than the audit-sink argument

Core now does record-and-flag: ParseDelegationChain never fails, sets malformed: true plus a low-cardinality malformedReason enum (act_not_object, nested_act_not_object, iss_not_string, sub_not_string), and keeps every hop parsed before the violation. Sentinel errors are gone. The evidence stack, because this should be a settled question rather than a taste call:

• MITRE ATT&CK T1562.006 (Indicator Blocking): if malformed act suppresses or degrades the record, anyone who can influence token content gets a remote, unprivileged primitive against the audit trail. This is the decisive argument.
• CWE-223 / CWE-778 make omission the weakness; NIST AU-3(f) requires records to identify the subjects with no "only if parseable" qualifier; UK DPA 2018 s.62 requires identity "so far as possible" — best-effort attribution, record still mandatory.
• RFC 8693 §4.1's MUST draws the line for us: only top-level claims + the current actor may inform access control; prior actors are "informational only". A malformed chain therefore cannot legitimately fail authorization — being recorded is its only purpose.
• RFC 9413 (the anti-Postel document) forbids silent tolerance, not recording: a loud flag satisfies it; silent dropping violates it twice over ("hiding the consequences of protocol variations… can conceal bugs"). The strictness it asks for belongs at the sender — i.e. Auth server can mint an act claim that violates RFC 8693 §4.1 toolhive#6093 should be fixed by rejecting at mint (invalid_grant, consistent with the adjacent depth check). Tolerant reader, strict writer; and with that split, malformed: true in our own logs becomes a high-signal alert that an issuer is buggy.
• Precedent: syslog RFC 3164 §4.3.3 (the canonical record-and-flag algorithm), CloudTrail's first-class userIdentity.type: "Unknown", ECS event.outcome: unknown, OTel's otel.metric.overflow sentinel.

The reason field is an enum rather than free text per OTel error.type guidance (low cardinality → safe as a metric/alert dimension). That's also my suggestion for your Malformed bool during migration: keep the bool, add the reason.

2. act_claims: staying with never-serialize, now with citations

NIST AU-3(1)/(3) (limit audit-record content to what's explicitly needed; limit PII to risk-assessed elements), OTel's header-capture rule ("instrumentations SHOULD require explicit configuration of which headers are captured" — JWT claims and HTTP headers are the same threat class: unbounded attacker-influenceable string maps), and the K8s Metadata audit level as the production allowlist precedent. One more you'll enjoy: attacker-controlled claim names emitted as dynamic log fields can exhaust Elasticsearch's default 1000-field mapping limit and fail ingestion for the whole index — a second indicator-blocking primitive that act_claims passthrough exposes and never-serialize closes.

Promoted iss stays: OIDC Core §5.7 makes (iss, sub) the only MUST-level stable identifier, and draft-mw-oauth-actor-chain (the current actor-chain standardization vehicle) encodes hops as exactly {iss, sub}.

3. Your findings checklist — all landed in 4aad6c9

• "chain": [] vs null: always allocated now; pinned by a test asserting the emitted bytes. Your prediction was right that our old hard-error path made it unreachable — the flag path makes it reachable, so it's pinned.
• Emitted-JSON key assertions: added — tests assert the key sets of the delegation object and hops from marshaled bytes, so a tag rename cannot pass green.
• Non-string sub: flags sub_not_string and retains the raw value in the in-memory extra (never serialized) — no more anonymous-actor-with-no-trace. Same treatment for iss.
• Named map types: reflection fallback accepts jwt.MapClaims-shaped input at any nesting level.
• Cap boundaries: kept (at-cap, cap+1, depth-1000 exact counts), plus malformed-past-cap now matches your #6096 semantics: flags malformed, does not count the unreadable node as omitted.
• Emitters: core has exactly one (LogTo). Related divergence fixed on our side: slog output now marshals the same shape as the event JSON (single slog.Any, chain array — the hop_N groups are gone), so one parser serves both wire forms. Worth noting for migration: toolhive currently spells the same field three ways (delegationChain JSON tag, delegation_chain slog key, delegation_chain on PrincipalInfo vs delegationChain in Identity.MarshalJSON) — collapsing those to one spelling is part of the win.
• Depth 10 vs 16: kept 16 with the distinction documented on the constant, exactly as you suggested — parse-time defensive ceiling deliberately above any mint cap, so truncated: true remains a "foreign token" signal for a consumer passing its mint cap. FWIW 10 as an enforcement default now has a citable source: draft-mw-spice-actor-chain §21.9 (RECOMMENDED default 10).

4. Ordering: outermost-first, now deliberate rather than coincidental

We already agreed, but the survey turned up a real hazard worth documenting (and I did, on the type): every append-style flat chain out there (Envoy XFCC, GCP delegates[], SPICE ach) runs the opposite direction (original-first). Current-actor-first is still right for this type: it matches RFC 8693's nested read order, and — decisive for an audit record with truncation — it keeps Chain[0] stable under truncation, since truncation drops inner hops. Original-first ordering would let index 0 silently hold a mid-chain actor on truncated chains. (Amusingly, #6035's acceptance criteria ask for chronological original-first while its own proposal says outermost-first; the RFC's order won.)

5. Migration path for toolhive

Happy to drive this. It's smaller than the table suggests because #6096 already moved your parser to value-return + Malformed — semantically we've now converged on your model. Remaining deltas: renames (delegationChain→delegation, actors→chain, dropped_count→omitted incl. dropping its omitempty), promote iss out of act_claims, drop act_claims serialization entirely (which also makes the internalClaims/tsid per-hop filter moot — the leak becomes structurally impossible), adopt malformedReason, unify the three wire spellings, emit the chain via marshaling in LogTo, and keep passing maxDepth=10 at the call sites. Plus fixing #6093 at mint per point 1. docs/middleware.md examples get rewritten once, before any consumer exists (#6082).

[jhrozek]

Re-review of 4aad6c9 + c7d4de0 — nearly all of it lands, one new regression

I re-checked every finding against the code rather than the summary, and mutation-tested the parts that claimed new coverage. Verified fixed:

finding	evidence
malformed → record-and-flag	ParseDelegationChain has no error return, sentinels gone, MalformedReason enum, hops before the violation preserved
depth invariance	h1→h2→h3→act:"garbage" now yields a record at both maxDepth=2 (2 hops, omitted=1, malformed) and maxDepth=16 (3 hops, malformed) — previously a record vs. a hard error
inflated omitted	countRemainingHops returns (int, bool); the non-object is flagged, not counted
"chain": null	always allocated, pinned from marshaled bytes
json: keys unasserted	re-ran all three mutations — chain→actors (7 failures), omitted→dropped_count,omitempty (fails), flagMalformed neutered (fails). Was green on all three before
non-string iss/sub	flags iss_not_string/sub_not_string, raw value retained in the in-memory extra
act: null	terminates the chain
dead branches / unused import	gone — 100% statement coverage on all eight functions in delegation.go, 99.0% package
§6 miscitation, actor/subject conflation	corrected, and OIDC Core §5.7 is the right authority for (iss, sub)
depth-cap rationale (c7d4de0)	now says exactly what it does — parse ceiling on retained size, explicitly not work

task lint clean, go test -race ./audit/ green at c7d4de0.

Two things I'll concede outright. The MITRE T1562.006 framing for record-and-flag is a better argument than the audit-fidelity one I made — "malformed input can suppress the audit trail" is a security property, not a quality preference, and malformedReason as a low-cardinality enum makes it alertable in a way a bare bool isn't. And the ordering-rationale note about Envoy XFCC / GCP delegates[] running original-first is a hazard I did not flag and should have.

The one blocker is a regression introduced by the shape unification. Passing *DelegationChain straight to slog.Any is PII-safe only because JSONHandler happens to route through encoding/json, which skips unexported fields. TextHandler formats with %+v and prints extra in full — including the tsid-class internal claim that never-serialize exists to make unrepresentable. Verified at c7d4de0, details inline. The fix is ~20 lines and keeps both wire shapes identical; I applied it and the suggested test locally before proposing them.

Marking this REQUEST_CHANGES only for that one item — everything else in the delegation work is in better shape than when I first read it.

[jhrozek]

✅ Everything I raised is resolved — approving

Re-verified eb00bd0 by exercising the code rather than reading the diff:

item	verification
LogValue PII guard	text handler leak=false; mutation — rename LogValue away and the suite fails on all three new assertions
slog/JSON parity, incl. omitempty	delegation object byte-identical between json.Marshal(event) and JSONHandler across six fixtures: normal, empty hop, iss-only, non-string sub, non-object, 20-deep truncated
text-handler test	adopted verbatim; it is what the mutation trips
doc.go wording	package doc and the constant now say the same thing
IsEmpty deleted	no references left; test renamed to TestDelegationChain_IsZero

task lint clean, go test -race ./audit/ green, coverage 99.1% — the sole uncovered block is the pre-existing e.Data path in LogTo, untouched here.

Putting LogValue on the type in delegation.go rather than next to LogTo was the right call, and I'd have argued for it if you hadn't done it — it's the type's contract, not the event's.

Two notes on the earlier rounds, since you asked directly. On (chain, error) vs. no error at all: your (b) is the argument that settles it — if err != nil { return } at a call site is exactly the silent-drop this feature exists to prevent, and a contract that's easy to hold wrong in an audit path is a worse contract. With flag+reason on the struct there is genuinely nothing left to branch on. The %T fidelity loss is real but cheap next to that, and malformedReason recovers most of it as a bounded enum. Keep it as is. On flagging a malformed tail past the cap: agreed, and it's the fix that restored depth invariance — the same token now produces a record with the same malformed signal at any maxDepth, where before it was a record at 2 and a hard error at 16.

One residual below, explicitly non-blocking: the guard is airtight for the shape LogTo passes, and porous for three shapes a consumer can construct. Fine to land as-is and follow up, or take the two suggestions.

[JAORMX]

@jhrozek the 4eeaad5 push auto-dismissed your approval — it contains only your two non-blocking suggestions applied verbatim (String() on the hop + the LogValue nil guard) plus tests pinning both. CI is green across all five checks and every thread is resolved. Mind re-approving so this can land?

[jhrozek]

✅ Re-approving 4eeaad5

Both hardening suggestions landed, and you pinned each one rather than just applying it — TestDelegatedActor_FmtCannotLeakExtra and TestDelegationChain_LogValueNilSafe. Mutation-checked all three guards; each fails the suite when removed:

guard removed	result
DelegatedActor.String()	❌ FAIL
LogValue nil guard	❌ FAIL
LogValue itself (the earlier fix)	❌ FAIL

Independent behavioural check at this SHA, not relying on the new tests:

fmt  %+v hop / %v value / %+v pointer   leak=false   {iss:i sub:s}
slog pointer / value / bare hop         leak=false
slog nil pointer                        no attribute emitted
slog delegation == json delegation      true

task lint clean, go test -race ./audit/ green, coverage 99.1% with String and LogValue at 100% — the only uncovered block remains the pre-existing e.Data path in LogTo, untouched by this PR.

Nothing outstanding from my side. Three rounds' worth of blocking items — the schema divergence with the merged toolhive#6046, the (nil, err) contract that let a malformed act erase the chain from the record, and the TextHandler PII leak from the shape unification — are all resolved, each with a test that fails if the fix is reverted.