Skip to content

Fix the HMAC key derivation, define key_id, and record an unresolved request_id_ref - #272

Open
rocklambros wants to merge 2 commits into
integrationfrom
spec/118-hmac-key-derivation
Open

Fix the HMAC key derivation, define key_id, and record an unresolved request_id_ref#272
rocklambros wants to merge 2 commits into
integrationfrom
spec/118-hmac-key-derivation

Conversation

@rocklambros

@rocklambros rocklambros commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

What changed

§10 said the HMAC key is "HKDF-derived" and stopped there, so two implementations could hold the same secret and still fail to verify each other. This PR writes down the parameters all four implementations already use, defines what key_id means for HMAC (Ariel's rule from issue #118), and adds the request_id_ref rule settled on issue #118 in September. The reference Guardian now follows that rule.

Which issue does this implement

Closes #118

  • This is an editorial correction (typo, grammar, link, formatting) with no change in
    meaning, so it needs no issue

Base branch

  • integration, because this touches the specification, schemas, a reference
    implementation, an adapter, tests, or CI
  • main, because every changed path is on the documentation lane allowlist in
    CONTRIBUTING.md

Type of change

  • Specification change (schema, hooks, events, AgBOM)
  • Reference implementation or adapter
  • Documentation
  • Tooling or CI
  • Governance (licensing, security policy, contributor docs)

Specification changes

Breaking for implementers? Not for anything merged today. The HKDF parameters are the ones the TypeScript, Go and Python implementations already use. The key_id rule changes the open ports, though. PR #229 ties key_id to session_id, PR #22 hardcodes "default", and PR #169 and PR #183 ship default labels. Each was asked on issue #118 to read the label from config and refuse one it doesn't know. The schema changes are descriptions only.

Here's what's in it:

  • §10, HMAC key derivation:
    • HKDF-SHA256 with no salt, info set to the UTF-8 session_id, and a 32-byte key.
    • A worked example. Python's standard library and Node's hkdfSync agree on its key, and tests/test_hmac_key_derivation.py recomputes it from the stated parameters, so a typo in the example fails the build.
  • §10, key_id for HMAC:
    • An opaque label the deployment chooses, naming one configured secret, with no reserved values.
    • A verifier rejects an unknown key_id and never falls back to another secret, and key_id is never derived from the secret.
    • The example is "2026-10", which gives rotation something to hang on. One sentence is my addition, so please check it: "A Guardian signs its response with the secret, and the key_id, that the request named." Rotation doesn't work without it.
  • hooks.md, toolCallResult:
    • A Guardian SHOULD resolve request_id_ref against the session's toolCallRequest records.
    • It MUST record one it can't resolve.
    • Denying stays a policy call.
  • Reference Guardian:
    • An unresolved request_id_ref now writes a finding line to the envelope log, and the decision doesn't change.
    • It resolves only within the session, because request_id is unique only there.
    • The Inspector shows these as ! FINDING. Before this, it would have warned about each one and dropped it.

I tested this

  • I synced my branch with the base branch before opening this
  • uv run pytest -v passes on my machine
  • uv run mkdocs build --strict passes on my machine

From reference-implementations/agt/, bun run typecheck is clean and bun test passes (1118). The new Guardian tests drive a running Guardian, and I watched each one fail before the change. They cover:

  • an unresolved reference, which gets a finding and an unchanged decision;
  • a reference to the session's own request, which gets nothing;
  • a result with no reference, which gets nothing;
  • a reference to a request in another session, which counts as unresolved.

Checklist

  • Commits are signed off with git commit -s (required by the DCO)
  • Prose follows STYLE.md
  • No secrets, tokens, or internal URLs in the diff

Security

  • This change has no security impact

It closes two gaps. Without fixed HKDF parameters, a second implementation had to match our code, not our spec, to verify ACS-Core signatures. Without a key_id rule, nothing stopped a verifier from falling back to some other secret, and that's exactly the habit a "default" label invites.

@rocklambros
Fix the HMAC key derivation, define key_id, and say how request_id_re…
b5b728c 
…f resolves

Section 10 now fixes the HKDF parameters every implementation already uses:
SHA-256, no salt, the UTF-8 session_id as info, and a 32-byte key, with a
worked example that a test recomputes. key_id for HMAC is an opaque label the
deployment chooses for one configured secret; a verifier rejects an unknown
label and never falls back. A Guardian SHOULD resolve request_id_ref and MUST
record one it cannot resolve.

Signed-off-by: rocklambros <rock@rockcyber.com>
@rocklambros
Record a request_id_ref the reference Guardian cannot resolve
f2304bf 
A toolCallResult whose request_id_ref names no toolCallRequest in the
session's chain now gets a finding line in the envelope log, which is the
Guardian's audit record. The decision is unchanged, because hooks.md leaves
denial to policy. The Inspector accepts finding lines and labels them, where
before it would have warned and dropped them.

Signed-off-by: rocklambros <rock@rockcyber.com>
@rocklambros rocklambros self-assigned this Oct 13, 2026
@github-project-automation github-project-automation Bot moved this to Needs triage in ACS Project Tracker Oct 13, 2026
@github-actions

github-actions Bot commented Oct 13, 2026

Copy link
Copy Markdown

This pull request's description says it closes #118, which is on the project roadmap. When it merges, GitHub closes that issue and the roadmap counts it as delivered. Please confirm the change delivers the whole issue before merging.

@rocklambros

rocklambros commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

@afogel @bar-capsule, I found that the signature section of the spec (Section 10, "Cryptographic Signatures") never says exactly how to derive the HMAC key or what goes in the key_id field, while investigating PR #229, where the TypeScript Guardian would reject every signature from PR #22's Python adapters. So I opened this PR to spell out the key derivation settings all our implementations already use, Ariel's key_id rule, and the request_id_ref rule we settled in September. It allows independent implementations to verify each other's signatures and lets a deployment rotate its secret without breaking things.

Two quick asks. Ariel, please check the one sentence I added next to your rule: a Guardian signs its response with the key_id the request named. Bar, please check the reference Guardian change, which adds a new finding line to envelopes.jsonl. CI is green, and either of your approvals lands it.

No rush on this one, since it isn't blocking Friday. PR #258 and PR #261 come first.

@acs-board-reconciler acs-board-reconciler Bot moved this from Needs triage to In progress in ACS Project Tracker Oct 14, 2026
@rocklambros rocklambros mentioned this pull request Oct 14, 2026
Open
7 of 14 tasks
President added a commit to disciplinedware/agent-control-standard that referenced this pull request Oct 15, 2026
@President
Answer the 12 October review of the Go Guardian
e233a35 
The embedded schemas follow integration after GenAI-Security-Project#63, GenAI-Security-Project#258 and GenAI-Security-Project#261.

The parity recording stores the git object IDs of the deciding TypeScript
paths instead of a commit, so a squash merge no longer makes
TestRecordingIsCurrent fail. The paths include agt.lock and the
agt-bridge package.json, which pin the AGT commit and SDK. The recorder
and the test read them from one list; the test fails when the recording
names other paths or was made from another corpus, and the recorder
refuses uncommitted changes to them. A test checks that the Go workflow
runs on every deciding path. agt/README.md tells a TypeScript contributor
when to re-record.

A test requires agt-native.lock and agt.lock to pin the same AGT repo
and commit. The native build fails when AGT at that commit changes
ffi.rs, which agteval/acs.h is written from, or fetches another opa than
agt-native.lock pins.

Under the agt evaluator, policy.opa_timeout must be shorter than
protocol.decision_timeout; guardian.yaml ships 4s. Otherwise a slow opa
lets the agent stop waiting before AGT's deny arrives (GenAI-Security-Project#269).

make load measures either evaluator under concurrent clients, selected
by EVALUATOR like the other targets. Under agt each decision runs one opa
process; on a 20-core host decisions stop rising
near 16 concurrent ones, and at 256 they reach the 5s timeout.
guardian.yaml now ships limits.max_engine_calls 32, and docs/load.md
records the results. The load run found that an engine call gave its slot
back only after its answer was sent, so a client's next step could be
refused engine_saturated with no call running; callEngine now releases
the slot first, also after a panic.

When the host adapter's audit log cannot be written, the record goes to
stderr as one JSON line with the final disposition, and the refusal's
reasoning says whether the record went to stderr or nowhere. The audit file's close error now
counts. The Codex and OpenCode hooks pass their own stderr. The §6.3 row
says that ALLOW at a hook that permits no DENY is an interpretation
(GenAI-Security-Project#270), and the docs say only the adapter library reaches those hooks.

Dependabot proposes Go updates, one pull request per dependency across
both modules. The Go workflow runs weekly, so govulncheck sees advisories
for the toolchain that Dependabot does not move, triggers on the
agt-bridge package.json, and no longer fetches full history. A test
checks that the method table permits DENY exactly where hooks.md does,
and fails on a Decision line it cannot classify.
The coverage floor counts every package except commands, examples and
test helpers.

The Guardian meets the expectations GenAI-Security-Project#256 sets for reference
implementations, mapped to tests in docs/conformance.md. audit.max_file_bytes
and audit.max_files rotate each audit file; guardian.yaml keeps at most
1 GiB per log; max_file_bytes needs max_files of at least 1, so a
rotation never deletes the live log. make load can set the rotation and
fails if the log drops, splits or reorders a record or exceeds its bound;
docs/load.md records a run and the cost of long sessions. storage.provider names the session store. The HMAC secret
file holds base64 of at least 32 bytes. key_id is a configured label with
no default, an unknown key_id is refused (GenAI-Security-Project#118, GenAI-Security-Project#272), and the hooks and
the conformance harness take it as a flag. A toolCallResult whose
request_id_ref matches no request is recorded (GenAI-Security-Project#272).

Decisions from other pull requests that the Go code contradicted:
- A shell command's URL ends only at ASCII whitespace; U+FEFF or U+00A0
  in the userinfo no longer hides the host curl reaches (GenAI-Security-Project#268).
- Versions are compatible when they share X.Y under 0.x; the handshake
  negotiates only such a version, and a step must match it (GenAI-Security-Project#196, GenAI-Security-Project#255).
- A policy target that is not a string is denied evaluation_failed; the
  TypeScript fix is in GenAI-Security-Project#229, so two recorded cases are listed as ahead of
  the recording until it is re-recorded.
- The host adapter records every decision failure under either posture,
  with its class, handshake failures included (GenAI-Security-Project#37).
- The host adapter denies under any posture a signed error other than
  -32603, an unverified answer and a malformed answer; the docs mark this
  as an interpretation of §6.4 ahead of GenAI-Security-Project#32, which is open. The version
  rule is marked the same way against §3.
- The audit directory is 0700; a symlinked audit path or an audit file
  open to group or others is refused (GenAI-Security-Project#71).

Claude-Session: https://claude.ai/code/session_01DFdYnoBitHhPcUSq7wMWs1
Signed-off-by: Konstantin Trunin <konstantin@trunin.com>
@President President mentioned this pull request Oct 15, 2026
Open
@rocklambros rocklambros mentioned this pull request Oct 15, 2026
Open
artmaro added a commit to artmaro/agent-control-standard that referenced this pull request Oct 17, 2026
@artmaro
Point the signing section at section 10 instead of restating it
46a9193 
Review on this pull request asked to link section 10 rather than restate
the HMAC key derivation and the meaning of key_id, so the README cannot
drift when section 10 changes. GenAI-Security-Project#272 adds those rules to section 10.

The section keeps only what section 10 does not define: how key material
is encoded and read, that it stays out of code and logs, and that each
implementation documents how to configure and replace it.

Part of GenAI-Security-Project#252

Signed-off-by: Evgeny Kokuykin <artmaro@gmail.com>
@bar-capsule

bar-capsule commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

LGTM, approved. The finding line looks good and the inspector shows it fine.
FYI, I ran the HKDF example against the #22 adapter and got the same key. I'll update #22 to take key_id from config instead of hardcoding "default".

@imran-siddique

imran-siddique commented Oct 19, 2026

Copy link
Copy Markdown

With info set to session_id alone, both directions use one key, and the Observed Agent derives it too. §8.6 (specification.md:313 at 9c05f04) names only the Guardian as the key holder who can re-sign a head. The agent can too, and can mint a signed ALLOW. A direction label in info will not fix that, since both sides hold the secret. One sentence in §8.6 would: under HMAC, a Guardian decision is not attributable to the Guardian against the agent.

This branch has not been deployed

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

Reviewers

@afogel afogel Awaiting requested review from afogel afogel is a code owner
@bar-capsule bar-capsule Awaiting requested review from bar-capsule bar-capsule is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@rocklambros rocklambros

Labels

None yet

Projects

ACS Project Tracker
Status: In progress

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Proposal] Clarify request_id_ref semantics and ACS-Core HKDF parameters

3 participants

@rocklambros @bar-capsule @imran-siddique