Skip to content

Add adapters/ with Claude Code, Cursor, and NAT reference implementations - #22

Open
bar-capsule wants to merge 1 commit into
GenAI-Security-Project:mainfrom
bar-capsule:bar/adapters
Open

Add adapters/ with Claude Code, Cursor, and NAT reference implementations#22
bar-capsule wants to merge 1 commit into
GenAI-Security-Project:mainfrom
bar-capsule:bar/adapters

Conversation

@bar-capsule

@bar-capsule bar-capsule commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Introduces a top-level adapters/ directory holding reference implementations that wire popular agent frameworks to an ACS Guardian through configuration only, with no agent code changes. Ships three working adapters with passing tests for each.

What lands

Adapter Status Mapping Working adapter Tests Live verification
adapters/claude-code/ Reference implementation 13 unit + 2 automated live tests ✓ Automated: real claude --print round-trip, ALLOW + DENY paths (test_live_claude_code.py)
adapters/nat/ Reference implementation 7 integration + 5 live workflow tests ✓ Automated: exercises real function_middleware_invoke against nvidia-nat-core 1.7.0 (test_live_nat_workflow.py)
adapters/cursor/ Reference implementation 13 unit tests ✓ Manual reproduction procedure in tests/live_verification.md (Cursor is a desktop app with no documented headless mode)
adapters/example-guardian/ Shared test fixture n/a n/a n/a Used by all three adapters' integration tests

Total: 40 automated tests + 1 documented manual verification procedure, all passing.

The adapter pattern

ACS-Core specifies what a hook event looks like on the wire and what the Guardian's decision looks like coming back. It does not dictate how a framework physically wires the interception in. Each adapter demonstrates the boundary choice for its framework:

Adapter Interception mechanism Event dispatch Block mechanism
Claude Code Shell command per hook (settings.json) Event type in stdin JSON hookSpecificOutput.permissionDecision
Cursor Shell command per hook (hooks.json) Event type as argv[1] permission (top-level, per-event) + exit code 2
NAT In-process Python FunctionMiddleware class NAT's middleware pipeline Raise ACSGuardianDenied (NAT 1.7.0) or InvocationAction.SKIP (NAT dev)

All three send the same ACS JSON-RPC shape to the Guardian. The example Guardian (adapters/example-guardian/example_guardian.py) is shared across all adapters — same wire format regardless of which framework emits the event.

The top-level adapters/README.md contains a step-by-step walkthrough with concrete JSON payloads at each step, a cross-adapter comparison table, and a flow diagram. Read that first.

Claude Code adapter

Wire it up by editing ~/.claude/settings.json (see settings.json.example); no code changes to your agent.

Live verification: tests/test_live_claude_code.py spawns claude --print in a sub-process with a project-level settings.json wiring the adapter into PreToolUse. Tests both:

  • ALLOW path: benign echo command runs and the marker string appears in Claude Code's output.
  • DENY path: Guardian's destructive-Bash policy denies; Claude Code's response surfaces the block.

Both passing in ~18s.

Schema corrections discovered via the live test (real Claude Code differs from public docs):

  • PreToolUse output uses hookSpecificOutput.permissionDecision = "deny", NOT top-level decision: "block".
  • PostToolUse field is tool_response (object), NOT tool_output (string).
  • Real payloads include tool_use_id, effort, duration_ms not mentioned in public docs.

NAT adapter (NVIDIA Agent Toolkit)

Real NAT middleware class. Installs via pip install nvidia-nat-core, configured in NAT workflow YAML:

middleware:
  acs:
    _type: acs_guardian
    guardian_url: http://127.0.0.1:8787/acs
    default_deny: true

workflow:
  _type: react_agent
  middleware: [acs]

Live verification: tests/test_live_nat_workflow.py exercises NAT's actual orchestration method (FunctionMiddleware.function_middleware_invoke) — the same code path NAT's runtime calls when a function with middleware is invoked. Tests prove the load-bearing property: when the Guardian denies, the target function does not execute (the test's side-effect counter stays at 0).

Covers: allow / deny / fail-closed / fail-open. 5/5 passing against nvidia-nat-core 1.7.0.

Schema corrections discovered while building:

  • InvocationAction.SKIP is on the NAT dev branch, NOT in 1.7.0. Block by raising. Adapter feature-detects and prefers action-based path when available.
  • Middleware configs must inherit FunctionMiddlewareBaseConfig with name= class kwarg (NAT's TypedBaseModel registration). Plain Pydantic BaseModel fails on @register_middleware.

Cursor adapter

Real Cursor schema sourced from Cursor's own bundled ~/.cursor/skills-cursor/create-hook/SKILL.md. Maps all 20 documented Cursor hook events to ACS steps/* methods.

Cursor is a desktop application without a documented headless mode, so live verification is a documented manual procedure in tests/live_verification.md. The procedure has been run end-to-end (5+ hooks flowed through the adapter, zero adapter errors); captured payloads from that reproduction are not committed because Cursor's events include session-identifying fields. Anyone with Cursor installed can reproduce.

Why in-spec adapters/ and not separate repos

Single repo for spec + reference implementations on the first batch makes the spec evolve alongside the adapters that exercise it (the live tests on this PR found several real schema gaps between docs and actual behavior — that feedback loop is what makes the spec text trustworthy). When the pattern stabilizes and individual adapters need their own release cycle, splitting to separate repos is straightforward.

What this PR does NOT do

  • Does not modify any normative spec text. Adapters are reference implementations, not spec.
  • Does not introduce new profiles. The adapters exercise ACS-Core; profile-tier adapters (acs-audit, acs-crypto, etc.) layer on later.
  • Does not address ACS signing the outbound traffic. That's a deployment concern handled at the transport layer for these minimal adapters.

Running the tests

# Claude Code (requires `claude` CLI on PATH for the live tests)
cd adapters/claude-code && python3 -m unittest tests -v

# Cursor (unit tests only; manual reproduction per tests/live_verification.md)
cd adapters/cursor && python3 -m unittest tests -v

# NAT (requires pip install nvidia-nat-core)
cd adapters/nat && python -m unittest tests -v

🤖 Generated with Claude Code

@bar-capsule bar-capsule changed the title Add adapters/ with Claude Code reference implementation, NAT + Cursor scaffolds Add adapters/ with Claude Code, Cursor, and NAT reference implementations Jun 15, 2026

@rocklambros rocklambros left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bar-capsule I went deep on this branch before writing anything. Checked it out, ran the three suites, read each adapter against the v0.1.0 schemas. The config-only pattern is the right call, and the per-adapter docs are honest about what's deferred. I want to flag a gap before "reference implementation" sticks, because people copy reference adapters line for line, and a few of these would ride along into their deployments. Marking this request-changes so the wire-format and fail-open items get a look before merge, not as a veto on the direction.

I checked all of this against the open issues first (#10 through #19). Those are spec-level: capability resolution, HMAC key management, the conformance program. Nothing below is a dupe. This is adapter-implementation ground. Three of mine have a protocol-level twin and I'll point those out.

The one that matters most: the adapters don't emit the v0.1.0 wire format. In claude-code/acs_adapter.py the request puts acs_version, request_id, timestamp, and metadata at the top level, but request-envelope.json wants them inside params and sets additionalProperties: false. A schema-validating Guardian rejects every request. timestamp goes out as epoch millis (int(time.time() * 1000)) where the schema asks for an ISO-8601 string. The payload uses tool / name / arguments where the hook schema wants the payload wrapper with arguments as {value, provenance}. Same shape in the cursor and nat adapters. The tests stay green because example_guardian.py reads params.get("tool") too, so both sides agree with each other and disagree with the spec. Nothing validates an emitted envelope against acs_schema.json. One test that does would have caught all of it, and it's the highest-leverage thing you can add here.

The deny path fails open on anything it doesn't recognize. In translate_response, an unknown or empty decision returns {}, which Claude Code and Cursor both read as "proceed." ACS_DEFAULT_DENY only kicks in on an exception (Guardian unreachable), not on a Guardian that answers with a verdict the map doesn't know. So a v0.2 disposition, a typo, even a trailing space surviving .lower(), all proceed. NAT already does the right thing and blocks under default_deny. The other two should match it. One line each.

No signing at all (this one's adjacent to #11, not the same). The adapters don't HMAC the envelope, and the example Guardian neither verifies a signature nor checks for replays. The READMEs call this "deferred to transport," but conformance.md:28 lists the baseline signature as a Core MUST, and :67 says transport doesn't satisfy it. #11 is about key distribution and rotation once you're signing. This is the step before: the reference ships with no signing, so every copy starts from an unauthenticated channel. The default http://127.0.0.1:8787/acs in the config keeps that invisible until someone repoints the URL at another host.

Delegation walks around the gate. SubagentStart isn't in HOOK_MAP, Claude Code can't block on it anyway, and example_guardian.py allows Task by default. A subagent spawn isn't evaluated before it acts. That's the adapter-level version of #16, and it lands on the exact confused-deputy path the subagent hooks were promoted to cover. At minimum I'd surface it in mapping.md's "not mapped" list instead of leaving it silent.

On the tests: "40 tests, all passing" is true on your machine, not in CI. NAT's 12 tests skip when nvidia-nat-core isn't installed (I got Ran 12 tests in 0.000s, OK (skipped=12)), Cursor's live test is a skip placeholder, and no workflow runs the adapter tests at all (only sync_version.yml). Skips read as passes, so a regression that lets a denied call through lands green. There's no requirements.txt under adapters/, and the NAT install is unpinned (pip install nvidia-nat-core, no ==). The NAT deny test also catches ACSGuardianDenied and returns without asserting the call was actually aborted, so it passes as long as something raised. This cluster worries me second-most, because green tests on a security control are worse than no tests. Pin the dep, run the unit tests in CI, and have the deny tests assert a real side effect didn't happen (the file wasn't written, the counter stayed at 0).

Smaller stuff, and I'm less sure these are worth blocking on:

  • example_guardian.py's regex misses rm -fr /, rm --recursive --force /, rm -rf ~, and find / -delete. It's labeled illustrative so I won't die on this hill, but it's the only thing a newcomer can run on day one, so I'd make it harder to fool or louder about being a toy.
  • A PostToolUse deny can't undo a side effect that already ran (Claude Code's own hook docs say PostToolUse can't block the action). Cursor's beforeReadFile returns {}, so a denied file read still happens. The pre-hooks are the only real gate, and the docs should say so plainly.
  • NAT's _build_request isn't inside the try/except, so a non-serializable kwarg throws before default_deny can catch it. And post_invoke ignores a result-side deny.
  • mapping.md and the code disagree on the deny shape for non-PreToolUse hooks. The doc says {"continue": false, "stopReason": ...}, the code emits {"decision": "block", "reason": ...}. One is wrong against Claude Code's contract.

None of this changes my read on the direction. I like where this is going, and the cross-adapter table in the README is genuinely useful. I'd hold the "reference implementation" label until the envelope matches the schema and the deny paths fail closed, since those are the parts people copy without reading the footnotes. Happy to send a PR with the schema-validation test, or pair on the envelope fix if that's faster.

Tracking: I cross-linked the delegation gap onto #16 and the no-signing gap onto #11 so the protocol-level and adapter-level views sit together. The rest of the findings here are adapter-specific with no matching issue.

(cc @afogel since the envelope and signing points touch conformance.md.)

@rocklambros

Copy link
Copy Markdown
Contributor

Merge-order note: this should land after #21, and after the change-request items above are addressed.

  • The adapters' conformance posture marks system/ping and Wrapped MCP as "not implemented." That story only holds once Slim ACS-Core: relax MODIFY, system/ping, and wrapped MCP to SHOULD #21 relaxes those to SHOULD; before that the floor lists them as MUST, so the reference would be claiming conformance against a floor it doesn't meet.
  • This PR has changes requested (wire format and the fail-open deny path), so it shouldn't merge until those are resolved regardless of ordering.

No git conflict with #20 or #21 (this only touches adapters/), so the sequencing is about consistency, not rebasing. Overall order I'd suggest: #21, then #20, then #22. @bar-capsule

bar-capsule added a commit to bar-capsule/ACS that referenced this pull request Jun 17, 2026
Rock's PR GenAI-Security-Project#22 review caught that the three reference adapters and the
example Guardian shared a wire format that diverged from
specification/v0.1.0/request-envelope.json: acs_version / request_id /
timestamp / metadata at the envelope's top level instead of inside
params, timestamp as epoch milliseconds instead of ISO-8601 string,
tool payload missing the required payload wrapper, arguments not
wrapped per tool-call-request.json. Tests passed because the adapter
and the example Guardian agreed with each other; the canonical spec
was outside the test loop.

This commit:

- Restructures every adapter's envelope to nest the AcsParams fields
  inside params, ISO-8601 timestamps, metadata.{agent_id, session_id}
  populated, payload wrapped per the relevant hook schema, arguments
  wrapped as {value: ...} per tool-call-request.json:26-37.
- Updates example_guardian.py to read from params.payload, gate the
  Task subagent tool by default, and expand the destructive-Bash
  regex set (rm -fr, --recursive --force, ~, --no-preserve-root,
  find / -delete / -exec rm, chmod 777 on system paths).
- Fixes the fail-open-on-unknown-disposition bug in claude-code and
  cursor translate_response; NAT pre_invoke and post_invoke now
  default-deny on unknown verdicts.
- NAT post_invoke now honors a Guardian deny verdict by clearing
  context.output and setting acs_post_invoke_redacted, matching
  Specification §6.4's output-redaction gate.
- NAT _build_request is now inside the try/except in both pre_invoke
  and post_invoke so build errors apply the same fail posture as
  transport errors.
- Adds tests/test_envelope_schema.py to each adapter. These validate
  every adapter-emitted envelope and per-hook payload against the
  canonical v0.1.0 JSON schemas loaded from $ACS_SPEC_DIR. They are
  hard-FAIL if the schemas are missing — not skipped — because spec
  validation is non-negotiable.
- Adds .github/workflows/adapter_tests.yml to run the schema + round-
  trip + live tests per adapter on every push and PR, with the spec
  schemas pulled from upstream Agent-Control-Standard/ACS:main.
- Pins nvidia-nat-core==1.7.0 (adapters/nat/requirements.txt) and
  jsonschema>=4.20,<5 (adapters/requirements-test.txt).
- Updates each adapter's README conformance table to be MUST-honest
  against docs/spec/conformance.md: handshake, baseline HMAC-SHA256
  integrity, replay nonce, system/ping, wrapped MCP are now marked
  ✗ not implemented, with citations. The previous "deferred to
  transport layer" claim for baseline integrity was inconsistent with
  conformance.md:28 and :67.

Test counts (all pass, zero hidden skips):
  claude-code: 17 schema + 13 round-trip
  cursor:      36 schema + 13 round-trip
  nat:          6 schema + 7 round-trip + 5 live (NAT 1.7.0)

Co-Authored-By: Claude Opus 4.7 <[email protected]>
@bar-capsule
bar-capsule marked this pull request as draft June 18, 2026 08:34
bar-capsule added a commit to bar-capsule/ACS that referenced this pull request Jun 20, 2026
The original adapter_tests.yml had three bugs that made it
non-functional and reproduced the exact failure mode PR GenAI-Security-Project#22 review
flagged: tests "passing" via skips that operators read as green.

Fixes:
  - Python 3.13 → 3.12. NAT 1.7.0 has no 3.13 wheel; the install
    step would fail silently in skip mode on any NAT-dependent test.
  - Drop `example-guardian` from the matrix — no tests/ directory
    there; the matrix entry crashed on `unittest discover tests`.
  - Add the cross-adapter conformance suite (`adapters/test_acs_core_
    conformance.py`) as its own job. That 48-test file was previously
    not run by CI at all.

Skip handling, per Rock's "skips read as passes" point:
  - NAT job: NAT is installed (pinned `nvidia-nat-core==1.7.0` +
    matching `nvidia-nat-langchain`), so ANY skipped test means the
    test gating is buggy. Hard fail.
  - Conformance job: zero skips allowed — every ACS-Core MUST runs.
  - Other adapters: surface skips as warnings (Claude Code's live
    tests legitimately skip when the `claude` CLI isn't installed in
    CI; Cursor has a manual-procedure placeholder). Both intentional.

Now exercises ~190 tests on every push to adapters/ or
specification/, with the load-bearing security tests pinned and
required.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
@bar-capsule
bar-capsule marked this pull request as ready for review June 20, 2026 18:32
@bar-capsule

Copy link
Copy Markdown
Collaborator Author

Thanks @rocklambros for the thorough review! every item addressed.

Wire format vs request-envelope.json - Every emitted envelope is schema-validated against canonical request-envelope.json from $ACS_SPEC_DIR (not against fixtures, not against the example Guardian's shape). params wrapper, ISO-8601 timestamps, payload + {value, provenance} arg wrapping - all to spec.

Deny fails open on unknown - _fail(cause=…) taxonomy across all three adapters covering transport, adapter exception, signature failure, and 7 JSON-RPC error codes via shared guardian_error_cause(). Unknown disposition + ACS_DEFAULT_DENY=1 → block; otherwise an ACS_AUDIT fail_open_bypass event with the cause label.

Signing - HMAC-SHA256 across all three adapters via adapters/_common/. Every envelope signed; every response verified. SIGNATURE_INVALID / REPLAY_DETECTED / TIMESTAMP_OUT_OF_WINDOW each map to a distinct audit cause.

Subagent delegation - example_guardian gates Task by default (subagent_gated); opt-in via ACS_ALLOW_SUBAGENT=1. Each adapter's mapping.md lists where delegation hooks aren't honorable by the framework.

Tests on tests - Deny tests now assert the real side effect didn't happen, the way you described — counter checks plus, for NAT, a canary-file pattern (if rm -rf runs despite the deny, the canary file vanishes regardless of what the counter says). A real Vertex/Gemini react_agent run surfaced silent-bypass bugs the synthetic tests would have shipped; all have regression tests.

CI workflow - Pinned nvidia-nat-core==1.7.0 + nvidia-nat-langchain==1.7.0, Python 3.12, runs the per-adapter + conformance suites on every push to adapters/ or specification/, hard-fails on any skipped NAT or conformance test.

Smaller items: rm regex hardened (-rfv, -fr, --recursive --force, --no-preserve-root). READMEs say plainly that pre-hooks are the gate; post-hooks redact via output=None + audit. NAT _build_request moved inside try; post_invoke result-side deny propagates. mapping.md and code now agree on deny shape.

Would appreciate re-review when you have a window.

bar-capsule added a commit to bar-capsule/ACS that referenced this pull request Jun 21, 2026
…w Wrapped MCP claim

Two findings from Rock's review of PR GenAI-Security-Project#22:

P1.1 — Conformance CI fails for the right reason now.
  Without rfc3339-validator installed, jsonschema's date-time
  format checker silently no-ops and test_timestamp_is_iso8601
  false-passes (invalid "yesterday" passes validation; assertion
  sees an empty error list; CI shows green on a real wire-format
  bug). Pin rfc3339-validator in adapters/requirements-test.txt
  + add a fail-fast setUpClass guard that rejects any future
  degradation: if the date-time checker accepts "not-a-date",
  the whole conformance class refuses to run with a pointed
  error message.

P2.2 — Wrapped MCP claim narrowed.
  conformance.md:26 lists protocols/MCP/* as part of the Core
  baseline. Our Core10_WrappedMcp suite verifies the WIRE-FORMAT
  shape (envelope validates, Guardian returns a structured
  response, no crash) but not full MCP request wrapping; the
  reference Guardian routes incoming MCP through the standard
  toolCallRequest path with the tool name reflecting the MCP
  method. The module docstring and the top-level adapters/README
  now say plainly that a green run = "ACS-Core baseline minus
  full Wrapped MCP", not "the whole baseline". v0.2 deferral
  marked explicitly. Deployments needing full wrapping must
  extend the Guardian.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
bar-capsule added a commit to bar-capsule/ACS that referenced this pull request Jun 21, 2026
… to malformed base64

Reviewer caught that base64.b64decode() in verify_signature() ran
without exception handling. A malformed signature value
("not-base64", padding garbage, truncated input) raised
binascii.Error up to the Guardian's request handler, which only
catches GuardianError. Result: a bad signature tore down the
request path on the wire (uncaught exception, 500-class response)
instead of returning the spec's SIGNATURE_INVALID (-32004). Same
risk on the adapter side for malformed signed responses. Security
control was a DoS vector.

verify_signature() now catches binascii.Error / ValueError /
TypeError around the b64decode and returns False — the existing
caller chain (Guardian's check_signature, adapter's response
verification) then emits -32004 with cause=signature_invalid_*
and the audit event fires correctly.

Regression test: Item15_VerifySignatureRobustToMalformedBase64
exercises 7 forms of unparseable input (garbage, padding-only,
mid-string padding, oversized, empty); every one must return
False, none may raise.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
bar-capsule added a commit to bar-capsule/ACS that referenced this pull request Jun 21, 2026
…ation-only on every adapter

Reviewer caught that ACS-Core §hooks.md describes agentResponse as
decision-eligible (ALLOW / DENY / MODIFY), but every adapter
silently drops denies on the hook that produces it. Claude Code
maps Notification → agentResponse and returns {} on deny; Cursor
afterAgentResponse does the same; NAT lifecycle hooks are
fire-and-forget through the IntermediateStepManager subscription.

The framework constraint is real and not fixable in this PR:
  - Claude Code's Notification fires AFTER assistant message
    delivery — no veto path.
  - Cursor's afterAgentResponse fires AFTER the message — same.
  - NAT's IntermediateStepManager is a notification stream;
    subscriber callbacks cannot abort an event after it fires.

This commit makes the docs honest about that. Each adapter's
mapping.md now marks the relevant hook explicitly as
"observation-only" with an explanation of which framework
boundary blocks pre-delivery enforcement. The per-adapter
README conformance tables narrow the dispositions claim to
"ALLOW / DENY / MODIFY on pre-execution hooks" with a pointer to
mapping.md for lifecycle / post-execution observation-only
posture.

Also includes a hunk missed from the previous Wrapped MCP commit:
adapters/README.md's top-level claim now says "ACS-Core baseline
minus full Wrapped MCP" to match what the conformance suite
actually verifies.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
bar-capsule added a commit to bar-capsule/ACS that referenced this pull request Jun 21, 2026
…claim on post_invoke redaction

Reviewer caught that the Post-tool-deny-redaction row still said
post_invoke sets acs_post_invoke_redacted=True, contradicting the
code in adapters/nat/acs_adapter.py:294 / :717 — InvocationContext
is a strict Pydantic model and that extra attribute would crash.
The real redaction signal is context.output = None plus the
ACS_AUDIT post_invoke_redacted event. README now says so.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
Comment thread adapters/claude-code/README.md
Comment thread adapters/claude-code/README.md
Comment thread adapters/claude-code/README.md
Comment thread adapters/claude-code/README.md
Comment thread adapters/claude-code/README.md
Comment thread adapters/_common/e2e_report.py
Comment thread adapters/_common/test_harness.py
Comment thread adapters/_common/acs_common.py Outdated
Comment thread adapters/_common/acs_common.py Outdated
Comment thread adapters/_common/acs_common.py
@rocklambros

Copy link
Copy Markdown
Contributor

@bar-capsule

I ran this rather than just reading it, and the first thing I hit is that the CI job can't fail. Everything else I found is downstream of that in one way or another, so I'll start there.

This is a long comment because it's a 14,515-line PR that turns a docs repo into a code repo. The work is real and the threat model in adapters/SECURITY.md is better than most projects ship. That's precisely why the gaps matter, since a reader will trust this by default.

The CI gate reports success when the tests fail

Both test steps pipe into tee:

python -m unittest test_acs_core_conformance -v 2>&1 | tee out.log   # line 39
python -m unittest discover -v tests 2>&1 | tee out.log              # line 83

GitHub's implicit shell for run: is bash -e {0}, and pipefail only gets added when you write shell: bash explicitly. The workflow has no shell: key anywhere, so the pipeline's exit status is tee's, which is always 0.

I checked rather than assuming:

$ printf 'false 2>&1 | tee /dev/null\n' > v1.sh
$ bash -e v1.sh;           echo $?    # 0
$ bash -eo pipefail v1.sh; echo $?    # 1

The header comment says the workflow "addresses the 'skips read as passes' failure mode," and the skip grep at lines 41 and 91 does work. So right now the job catches "a test didn't run" and misses "a test ran and failed," which I'd argue is worse than no workflow, because the badge certifies something false.

shell: bash on both steps fixes it. Then re-run, because I think the suite is currently red (next item) and the green is hiding it.

The conformance suite can't run from a fresh clone

ACS_SPEC_DIR defaults to /tmp/acs-spec-source/specification/v0.1.0 in ten-plus places, including test_acs_core_conformance.py:66, _common/test_harness.py:148, example_guardian.py:97, three test_envelope_schema.py files, and all three e2e_check.py scripts. That path doesn't exist, and the canonical schemas ship in this repo at specification/v0.1.0/, one directory up from adapters/.

Running the documented command from adapters/README.md:22-25 gives Ran 6 tests ... FAILED (failures=1, errors=9). Pointing ACS_SPEC_DIR at the in-repo schemas gives Ran 48 tests ... OK.

Two things make this worse than a broken default. The documented remedy in claude-code/README.md:114-116 is git clone https://github.com/Agent-Control-Standard/ACS.git /tmp/acs-spec-source, which pins conformance to whatever's on remote main rather than the schemas in the PR under review. And in example_guardian.py, the missing directory leaves _SPEC_VALIDATION_AVAILABLE = False, so the Guardian silently skips envelope validation with no warning. Since check_replay and check_skew both return early on a falsy request_id or timestamp, an envelope that just omits both then sails past replay and freshness protection.

Path(__file__).resolve().parents[N] / "specification" / "v0.1.0" as the default, env var as override, and log loudly when validation is unavailable.

The Claude Code adapter drops subagent events entirely

I said something wrong about this earlier and want to correct it. mapping.md:17 documents a SubagentStop mapping, and I initially took that at face value. The code doesn't do it. HOOK_MAP at lines 64 to 72 has seven entries and contains neither subagent hook:

HOOK_MAP: dict[str, str] = {
    "SessionStart": ..., "SessionEnd": ..., "UserPromptSubmit": ...,
    "PreToolUse": ..., "PostToolUse": ..., "Notification": ..., "Stop": ...,
}

SubagentStop shows up only at line 77 in BLOCK_RESPONSE_HOOKS and line 358 in _TRANSLATORS, and both are unreachable because main() gates on HOOK_MAP first:

if hook_name not in HOOK_MAP:
    return 0

Feeding it a SubagentStop event with ACS_DEFAULT_DENY=1 and a dead Guardian gives exit 0, empty stdout, empty stderr. Not even a _fail() audit line. The dead entries are the part I'd flag hardest, because they make the file read as if the hook is handled, so anyone skimming for coverage gets a false yes.

Cursor has the mirror-image problem. acs_adapter.py:78-85 omits subagentStop deliberately and says why, which I think is honest and right given final_chain_hash is genuinely unknowable there. But hooks.json.example:54-58 still wires it, so the shipped config invokes a path that drops the event silently and skips ACS_DEFAULT_DENY=1 on the way out.

NAT's mapping.md:20 claims sub-workflows map to subagentStart/subagentStop. grep -i subagent nat/acs_adapter.py returns nothing, and line 23 of the same file quietly retracts it ("The minimal adapter ... emits steps/toolCallRequest and steps/toolCallResult for every wrapped function call"). Rows 17, 18, and 19 for knowledgeRetrieval, memoryContextRetrieval, and memoryStore are in the same position.

And even if all three emitted, example_guardian.py:157-164 puts both hooks in INFORMATIONAL_METHODS and returns allow unconditionally, so a spawn declaring intent_derivation: "fresh" from a constrained parent gets waved through. This matters a lot for #21, which promotes both hooks to MUST on confused-deputy grounds. I've left a longer note there about the spec side.

mapping.md inverts the fail posture

claude-code/mapping.md:43:

Decision honoring: the adapter's _fail() posture is deny by default (ACS_DEFAULT_DENY=1)

claude-code/acs_adapter.py:58:

DEFAULT_DENY = os.environ.get("ACS_DEFAULT_DENY", "0") == "1"

The default is fail-open. The doc says fail-closed. This is the row an adopter porting to a fourth framework reads to decide their own default, so it's wrong in the direction that overstates safety.

Five more in the same file, all verifiable against a line of code: :42 says baseline integrity isn't implemented (:259 calls sign_envelope), :38 says no handshake happens (:263-283 performs one), :27 and :28 document response shapes the code doesn't emit, and :40 describes a session-id derivation the adapter doesn't use. nat/mapping.md:84 and cursor/mapping.md:100 carry the same false "integrity deferred to transport layer" claim.

Nothing tests any mapping.md, so this drift is invisible to a green suite.

settings.json.example wires five hooks, not six

Wired: SessionStart, SessionEnd, UserPromptSubmit, PreToolUse, PostToolUse. Missing: Notification, which is the only event mapped to steps/agentResponse.

So the config an adopter copy-pastes fails even the pre-#21 six-hook minimum, while claude-code/README.md:280 claims all six and adapters/README.md:27 says "the 6 minimum hooks, all 5 dispositions."

The annoying part is that wire.py would have caught this. Line 319 emits NOTE: wiring a SUBSET of ACS-Core's 6 mandatory hooks. Missing: [...]. The checked-in example just wasn't produced by it. Also worth noting ACS_DEFAULT_DENY: "1" appears only on the PreToolUse block, while wire.py:101 classifies UserPromptSubmit as a gate hook too, so the two shipped wiring paths disagree about which hooks may fail open.

The e2e scripts print a conformance verdict they haven't earned

All three end with a variant of:

report.summary("YOUR CLAUDE CODE INSTALL IS ACS-CONFORMANT", width=68)

The suite's own docstring at lines 12 to 14 says a pass means conformant "minus full Wrapped MCP." claude-code/README.md:290 marks Wrapped MCP "✗ not implemented" in its own table. The banner carries neither caveat, and it's the artifact that ends up in a screenshot.

I'd scope it to what actually ran, something like ACS-CORE SMOKE PASS (4/4 scenarios; not a conformance certification), and reserve "conformant" for an instrument that enumerates the requirement set.

Signing has a canonicalization mismatch that resolves to fail-open

_common/acs_common.py:81-85 falls back to json.dumps(..., sort_keys=True, separators=(",", ":")) when rfc8785 is absent, and the docstring at 76 to 78 says that's "JCS-equivalent for all JSON shapes ACS envelopes carry." It isn't, for any float:

{"duration_ms": 1.0}    rfc8785: {"duration_ms":1}      fallback: {"duration_ms":1.0}
{"duration_ms": 1e16}   rfc8785: ...10000000000000000}  fallback: ...1e+16}
{"duration_ms": -0.0}   rfc8785: ...0}                  fallback: ...-0.0}

Key ordering diverges too. json.dumps sorts U+FFFF before U+10000, while RFC 8785 §3.2.3 sorts by UTF-16 code units and puts U+10000 first.

This is reachable, not theoretical. claude-code/acs_adapter.py:178-179 puts framework-supplied duration_ms straight into the signed payload. And the provisioning asymmetry is the default: nat/requirements.txt lists only nvidia-nat-core and ruamel.yaml, with rfc8785 appearing only in requirements-test.txt. So an adapter without it signs 1.0, a Guardian with it verifies 1, you get -32004, and per the next item that's a fail-open on every PostToolUse.

specification.md:290 says "Alternative canonicalization is not permitted in v0.1," so I'd delete the fallback and raise at import, make rfc8785 a hard runtime dependency everywhere, and add floats, -0.0, exponents, and a non-BMP key to the sample set. The current test skips itself when the divergence condition is present (test_edge_cases.py:60-63), which is the one case you want it to run.

Guardian rejection and Guardian failure are the same thing to the adapter

claude-code/acs_adapter.py:433-447 routes any "error" in response through _fail, and _common/acs_common.py:317-325 maps -32004 (bad signature), -32005 (replay), and -32600 (malformed envelope) into that same posture. Under the default ACS_DEFAULT_DENY=0 that means allow.

example_guardian.py:665-669 returns -32600 for any body over 1 MiB, before any policy code runs. So a prompt-injected agent issues Write with a 2 MB content argument, the envelope blows the cap, and the write proceeds ungoverned. Same primitive via -32005 by reusing a request_id, which acs_adapter.py:150 makes deterministic and guessable with uuid5(NAMESPACE_URL, f"claude-code:tool_use:{tool_use_id}").

SECURITY.md T6 frames the 1 MiB cap as DoS mitigation. It's also a policy-bypass primitive, because it sits upstream of evaluate_step.

The adapter is arguably conformant here, since §6.4 says an error without a decision follows the failure posture. I think that's a spec gap as much as a code one: ACS doesn't distinguish "the Guardian couldn't reach a verdict" from "the Guardian refused your envelope," and only the second is attacker-reachable. Codes that mean "alive and refused you" should fail closed regardless of posture.

The audit trail has nowhere to land

_common/acs_common.py:378 writes ACS_AUDIT <json> to sys.stderr. SECURITY.md:24 lists the audit log as an in-scope component. None of the three example configs route adapter stderr anywhere, and Claude Code doesn't surface hook stderr outside debug mode.

So when the Guardian dies, each hook invocation gets connection-refused, writes one line to a stream nobody collects, and exits 0. ping() exists at acs_common.py:508 and no adapter calls it, so nothing detects the outage either.

§6.4 offers a control-for-audit trade under disruption, and it's only a trade if the audit half lands somewhere durable. Right now it's control-for-nothing. ACS_AUDIT_FILE (append, 0600) plus setting it in all three example configs would close it.

Responses aren't bound to their requests

No adapter compares response["id"] to request["id"], or result["request_id"] to the one it sent. example_guardian.py:522-530 populates the field, so the binding is on the wire and just unread.

The signature is per-session, so a captured signed ALLOW for a benign ls verifies fine when replayed against rm -rf ~/. hmac.compare_digest is correct here and irrelevant, because the signature is genuine.

SECURITY.md T3 covers replay in the request direction only. Two lines per adapter, failing closed on mismatch.

Related and cheaper to hit: no ACS_HMAC_SECRET appears in either example config. With no secret, sign_envelope returns the envelope unsigned (acs_common.py:240-241) and verify_signature returns True (:274-276), so any local process that binds 127.0.0.1:8787 first becomes the Guardian and allows everything. That's an authentication failure rather than a confidentiality one, so SECURITY.md O1's "plaintext HTTP is a deployment concern" doesn't quite cover it.

Performance numbers

adapters/README.md:146 says "The whole round-trip is ~10 ms. The agent doesn't know any of this happened."

Seven runs of the real adapter with the Guardian port closed, so effectively zero network time, warm cache: median 93.9 ms, range 86.4 to 105.9. That's interpreter start plus the imports at lines 31 to 54, before the Guardian does anything. README.md:167 describes the spawn-per-event model correctly, so the model is right and the number just doesn't include it.

The slow-Guardian case is worse. Against a listener that accepts and never responds, three consecutive events measured 10.10s, 10.12s, 10.13s. That's the 5s handshake plus the 5s decision call, and it repeats every event because acs_common.py:491 guards the cache write behind if server_hello:, so a failed handshake never gets cached. There's no retry, no backoff, no circuit breaker (grep -rE "retry|backoff|circuit" on non-test adapter code is empty), and both timeouts are hardcoded.

Combine that with ACS_DEFAULT_DENY: "1" on PreToolUse in the example and a degraded Guardian means ten seconds of dead air followed by a denial, on every Read, every Bash, every Edit. There's no ACS_DISABLED check anywhere in main(), so recovery is hand-editing ~/.claude/settings.json mid-incident, per machine.

I'd replace the README number with a measured breakdown under stated conditions, note that NAT runs in-process and doesn't pay the spawn cost, and add an ACS_DISABLED early return documented as the incident procedure.

The conformance suite's central tests can't fail

A few of these, since the suite is the thing adopters are told to trust:

test_each_minimum_hook_returns_known_disposition (:437-449) asserts the decision is one of the five legal values. A Guardian that does return {"decision": "allow"} passes. The class docstring at 385 to 388 correctly identifies this risk and says the contradiction check catches it, but that check (:451-461) validates the broken payload locally via _validate_hook_payload and never sends it anywhere. So it tests that hooks/*.json constrains shape, which is a fine test of the schemas and no test of any Guardian.

Core04 drives two dispositions live and hand-synthesizes the other three. Line 545 says so directly: "Guardian doesn't emit modify in our example, so we construct one manually and validate it." That proves the wire can express MODIFY, not that anything implements it. Meanwhile the three adapters document three mutually incompatible substitutions (NAT sends ASK and DEFER to DENY, Cursor sends DEFER to ASK, Claude Code blocks both), none of which the suite covers.

Core10's MCP assertion is satisfied by example_guardian.py:576-577 denying protocols/MCP/* as an unknown method. Also, adapters/README.md:29 says MCP requests are "routed through the standard steps/toolCallRequest path" at the Guardian, and the test file at 1281 says the Guardian "falls through to unknown-method deny." The test file matches the code.

Core08 hardcodes HERE / "claude-code" / "acs_adapter.py" in five places, so Cursor and NAT have zero adapter coverage in the suite.

The load-bearing consequence: if you update the HOOKS list to eight for #21, both tests pass instantly and vacuously, because the Guardian returns allow from INFORMATIONAL_METHODS and the malformed check never leaves the process. The suite stays green and all three adapters stay non-conformant.

Spec citations by line number

The docstrings cite conformance.md by line (:4 says "lines 13-26", then :195, :255, :376, :465, :592, :694, :781, :871, :1209, :1276).

#21 is a same-line-count edit. Both files are 89 lines and the diff is exactly 19,20c19,20 and 25,26c25,26. So every citation stays numerically valid while four of them start quoting text that no longer exists, and a reviewer spot-checking "is conformance.md:19 still the hook-taxonomy bullet?" gets yes and moves on.

Compounding it, adapter_tests.yml triggers on adapters/**, specification/**, and itself. docs/** isn't there, so merging #21 runs zero tests.

Two fixes, both cheap. Add docs/spec/** to the trigger paths. Then add a test that pulls each docstring's quoted fragment and asserts it's a substring of the live conformance.md line. That's maybe thirty lines and it turns this whole class of drift into a red build.

Smaller things, grouped

NAT's post_invoke ignores default_deny. All three failure paths at nat/acs_adapter.py:680-711 return None with no posture check, while workflow.yml.example:10 sets default_deny: true. pre_invoke gets this right at :618-629 for the identical condition. So the output-redaction gate is unconditionally fail-open in the config you ship as the example. The audit type is also post_invoke_unreachable rather than fail_open_bypass, so any rule written against the documented label misses it.

An unknown or missing disposition allows with no audit in all three adapters. specification.md:158 says "Every step that proceeds without a decision MUST be recorded as an audit event," and the fail-open branch (which is the shipped default) has no coverage.

Observation-only hooks are advertised as enforceable. All three put them in methods_implemented, and example_guardian.py:488 echoes that straight back as methods_evaluated. A policy author writes a DENY rule on steps/agentResponse to stop exfiltration in an assistant message, and it silently never fires. The mapping.md files are honest about this in prose, but the handshake is what a Guardian reads at runtime and it has no way to mark a method observation-only.

The handshake is fetched, never verified, cached for an hour, and never read. ensure_session_handshake (acs_common.py:484-503) writes the ServerHello to disk without calling verify_signature, and every adapter discards the return value. So the Guardian's on_decision_failure, timeout_config, skew_window_ms, and profiles_accepted all have no effect, and the posture comes from a local env var instead. SECURITY.md T11 argues the advisory handshake prevents a signing-downgrade attack, and that reasoning is right for signing and backwards for posture: signing should be governed by whoever holds the key, and posture by whoever enforces policy.

A readable-but-unopenable secret file silently downgrades both ends to unsigned. acs_common.py:150-154 catches OSError and returns b"", sign_envelope then returns the envelope unsigned with no warning, and the Guardian's check_signature returns early in "local-dev mode." A volume that fails to mount turns off the only integrity control ACS-Core mandates, on both sides, silently. The startup guard at example_guardian.py:722 runs once and won't catch a mid-life loss.

The replay cache never shrinks. evict_old_request_ids deletes from st.seen_request_ids, then persist() at :287-290 merges from disk and re-adds exactly what was deleted. _load() re-parses the whole file per request, so cost grows quadratically until it trips the 5s timeout, which is a transport failure, which is a fail-open.

A renamed upstream hook silently disables enforcement. acs_adapter.py:404-405 and cursor/acs_adapter.py:542-543 both return 0 with no audit event. Every other bypass in those files emits one. Cursor renames beforeShellExecution, the adapter goes quiet, and the Guardian's chain looks like a quiet session rather than an ungoverned one. One audit_event("unmapped_hook_event", ...) line plus a KNOWN_UNMAPPED set to keep the intentional ones quiet.

defer passes through raw on the one hook that gates. PRETOOL_PERMISSION_MAP at :87-89 maps "defer": "defer", and I don't believe Claude Code's permissionDecision accepts that value. Worth verifying against the current hook schema rather than taking my word for it. Note the asymmetry inside the same file: _translate_posttool:330-332 and _translate_user_prompt:341 both convert defer to a block, and Cursor maps it to ask at cursor/acs_adapter.py:119 with the comment "no native defer," which looks like the right shape.

Dependencies float despite the header saying otherwise. adapter_tests.yml:5 says "Pinned dependencies," and four of six are ranges (jsonschema>=4.20,<5, rfc8785>=0.1,<1, rfc3339-validator>=0.1,<1, ruamel.yaml>=0.17). Both actions use mutable major tags rather than SHAs. The comment at 66 to 73 reasons carefully about why NAT must be pinned and then doesn't generalize it. There's also no permissions: block, unlike the sibling sync_version.yml which declares one.

free_port() is a bind-then-release race. _common/test_harness.py:60-64 closes the socket before returning, so the port is free between the return and the Guardian's bind. It'll flake under runner contention and surface as server not up on ..., which reads like a startup bug. The workflow also sets no timeout-minutes, so a hang burns the 360-minute default across five matrix jobs.

Disclosure path and ownership

adapters/SECURITY.md:227-230 says to "Open an issue ... with the security label, or email the maintainers listed in CODEOWNERS." CONTRIBUTING.md:65 says "Do not file public issues for security vulnerabilities." And there's no CODEOWNERS at root or in .github/.

So a researcher who finds a bypass in an adapter that's shell-spawned and holds the HMAC secret follows the nearest instruction file and posts a public zero-day. I'd delete that paragraph, point at CONTRIBUTING.md:65, and add a root SECURITY.md so GitHub surfaces the private path in the Security tab. The file is a threat model rather than a VDP, so THREAT_MODEL.md might be the more honest name.

On ownership: there's no CODEOWNERS, no .github/dependabot.yml, and the adapter dependencies live in loose requirements*.txt files outside pyproject.toml, so uv.lock covers none of them. nvidia-nat-core==1.7.0 pulls the LangChain tree, and when a CVE lands there nothing notifies anyone. CONTRIBUTING.md's Local Development section still lists three mkdocs commands and doesn't mention the test suite at all, so a new contributor won't know it exists.

There's also no version stamp on any adapter (grep -rn "__version__\|ADAPTER_VERSION" on adapters/ is empty) and no changelog. Since distribution is copy-paste, a defect here is an incident you can't scope: no way to ask an adopter what version they have, no way for them to find out, and no artifact to roll back to. acs_common.py:653-657 keeps a back-compat alias "so out-of-tree adapter forks aren't broken by the rename," so forks are clearly anticipated, and there's no mechanism to identify or update them.

Ordering

#21 rewrites the four conformance.md lines this suite cites, so I'd let that land first and update the citations here as part of the rebase. I've left notes on #21 and #20 with the details.

If it helps, the sequence I'd suggest is: fix the CI pipefail and ACS_SPEC_DIR first so the suite tells the truth about the rest, then re-run and see what's actually red before working through anything else here.

Last procedural note: CONTRIBUTING.md:42 asks for git commit -s and none of the 21 commits here have a sign-off. Probably worth a DCO check in CI rather than asking three PRs to rebase separately.

@rocklambros rocklambros mentioned this pull request Jul 28, 2026
3 tasks
bar-capsule added a commit to bar-capsule/ACS that referenced this pull request Aug 19, 2026
…d honest scoping (PR GenAI-Security-Project#22 review)

Squashed response to the full PR GenAI-Security-Project#22 review. Ships the three reference
adapters (Claude Code, Cursor, NAT) as ACS v0.1.0 EMISSION conformance:
the adapters, driven through their real production entry points, emit
schema-valid, signed Core traffic and honor the decisions the suite
tests — validated against the canonical schemas by an independent
oracle. It is deliberately NOT a full ACS-Core deployment-conformance
claim (that spans Guardian + framework wiring + production config and is
tracked as milestone GenAI-Security-Project#33).

Enforcement correctness
- Guardian REFUSALS (SIGNATURE_INVALID, REPLAY_DETECTED, TIMESTAMP_OUT_
  OF_WINDOW, malformed/oversized envelope) fail CLOSED regardless of
  posture — each is attacker-reachable, so routing them through the
  §6.4 fail-open posture was a bypass primitive. HTTP-layer refusals
  (413/400) and oversized envelopes are caught before the wire.
- Error responses are signed (schema + Guardian + adapters); an
  unsigned spoofable error under fail-open is an allow.
- Handshakes are signed (only system/ping is signature-exempt, §13);
  forward-compat accepts matching-major versions; malformed ClientHello
  and non-object/batch JSON-RPC return -32600 instead of crashing.
- Claude Task spawns emit steps/subagentStart (confused-deputy gate);
  Cursor default installer + example wire it fail-closed on BOTH
  ACS_DEFAULT_DENY=1 and failClosed:true; the Guardian gate is
  deny-by-default; no fabricated subagent lineage.
- ServerHello on_decision_failure is honored (most-restrictive-wins);
  handshake failures negative-cached; secret-file-unreadable and
  unsigned-mode are loud, not silent; rfc8785 is a hard dependency.

Emission conformance suite
- CaptureGuardian oracle validates the exact bytes each production
  adapter sends (real subprocess for Claude/Cursor; real middleware
  pre/post_invoke + lifecycle observer for NAT) against the canonical
  schemas, with an INDEPENDENT HKDF+HMAC+JCS signature verifier (not
  acs_common) pinned by a frozen known-answer vector, and proven
  non-vacuous by negative self-tests.
- Per-event: each Core method emitted once, envelope+payload valid,
  UUID/RFC3339/metadata/{value}-wrapper invariants. Per-session:
  handshake-once, unique request_ids, request_id_ref correlation,
  advertised==emitted (both directions).

Truthful gating
- One authoritative runner with platform selection
  (claude|cursor|nat), pass/skip/fail reported separately, exact
  (suite,test,reason) skip allowlist, fails on any UNEXPECTED skip.
  CI: shell:bash/pipefail (failing suites can't read green), DCO check,
  pinned deps + SHA-pinned actions, NAT installed so its tests run.
- Example configs are under test (the file people copy), not just
  wire.py output.

De-duplication
- Emission is the sole schema-validation owner; the double-validating
  build_request() schema tests were removed after moving every mapped
  native event into the emission matrix (coverage verified preserved).
  Suite reflects distinct assertions, not overlapping layers.

Docs
- Claim scoped to emission conformance throughout; hard-coded test
  counts removed; Wrapped-MCP kept as shape-only with its Core status
  flagged as a pending spec-owner decision.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Signed-off-by: Bar Kaduri <[email protected]>

@afogel afogel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Five findings from running the branch , with four of the five are one mistake in different places.

  1. A Guardian DENY with a trailing space executes the tool call. §6.4:158 says an arrived decision must be honored regardless of posture, and §6.3:146 already rules that an unusable one is a DENY.
  2. Failing closed on Stop and SubagentStop emits {"decision":"block"}, which Claude Code reads as "do not stop, continue". subagent-stop.json:5 marks both hooks not decision-eligible.
  3. settings.json.example passes all four settings through a per-hook "env" object that Claude Code does not implement, so a copied config runs unsigned, fail-open, with no audit sink. The test guarding that file asserts the same discarded field.
  4. Without rfc8785 the adapter exits 1 with empty stdout, which Claude Code treats as non-blocking, so the tool runs with no audit event and no regard for ACS_DEFAULT_DENY.
  5. The cached ServerHello is read without verifying its signature, so the observed agent flips its own fail posture by editing one word in a predictable path, or by touching an empty .failed marker.

The tests have a similar blind spot. The suite covers a Guardian that says nothing, thoroughly, across all three adapters. It does not cover a Guardian that says something unusable, or a framework that ignores what the config tried to say. §6.4:156 and §6.4:158 draw exactly that line, and the suggested expectations put it under test.

Two notes on scope, neither one a code comment.

§6.4:156 lists "malformed response" among the transport failures, which is where treating an unreadable decision as a decision failure finds cover. Line 158 then says an arrived decision must be honored regardless of posture. Those rub against each other for the case in finding 1. A clarifying sentence in §6.4 would turn that comment from an argument into a bug report, and settle the same question for whoever writes a fourth adapter. It fits with #32 rather than this PR.

This branch also carries three normative schema edits: subagent-stop.json drops final_chain_hash from required, response-envelope.json adds a signature to the error object, and otel-mapping.json moves an attribute to optional. The reasoning behind each looks sound. The PR description still says the change set touches no normative spec text. They want their own PR and a spec-owner decision, especially with #21, #31 and #32 open on adjacent ground.

Comment thread adapters/claude-code/acs_adapter.py Outdated
Comment thread adapters/test_acs_core_conformance.py
Comment thread adapters/claude-code/acs_adapter.py Outdated
Comment thread adapters/claude-code/acs_adapter.py
Comment thread adapters/claude-code/settings.json.example Outdated
Comment thread adapters/claude-code/tests/test_example_config.py Outdated
Comment thread adapters/claude-code/acs_adapter.py Outdated
Comment thread adapters/_common/acs_common.py
Comment thread adapters/_common/acs_common.py Outdated
bar-capsule added a commit to bar-capsule/ACS that referenced this pull request Aug 27, 2026
…apter gaps

One batch covering the five review findings from running the branch, the
adversarial/spec/host-contract audits they triggered, and the fixes those
audits surfaced. Every behavioral change ships with a regression test
that was verified to FAIL against the pre-change code (red-checked).

Review findings (all five fixed, with the reviewer's suggested tests):
- Unusable disposition fails closed: an arrived verdict that cannot be
  interpreted (padding, unknown value, non-string type, non-object
  modifications) is DENY on every adapter, never the fail posture; a
  never-raising normalize_decision() makes verdict reading total, and a
  translate-layer guard turns any future translation bug fail-closed.
- Stop/SubagentStop are audit-only: {"decision":"block"} there means
  "keep going" in Claude Code, so deny/decision-failure on those hooks
  records unenforceable_decision and emits nothing.
- settings.json.example regenerated from wire.py itself: all settings
  ride inline in the command string (Claude Code has no per-hook env
  field); the config test asserts the command string and pins
  example == wire.py output so they cannot drift again.
- Degraded bootstrap: a missing rfc8785 no longer dies as a silent
  exit-1 no-op — both shell adapters audit adapter_unavailable and honor
  ACS_DEFAULT_DENY with stdlib only; per-adapter requirements.txt pins
  rfc8785; NAT fails LOUDLY (UNGOVERNED banner + re-raise) since its
  in-process middleware cannot degrade per-hook.
- Handshake cache holds the whole signed ServerHello envelope and
  re-verifies the signature on every read; tampering is audited
  (handshake_cache_signature_invalid) and falls to re-handshake; the
  negative-cache branch is audited per §4.1.

Spec-conformance fixes (from re-deriving the normative requirements):
- Negotiated timeout honored: the ServerHello's timeout_config.default_ms
  replaces the hardcoded 5s deadline on all three adapters; a decision
  timeout is audited distinctly (decision_timeout) from an unreachable
  Guardian.
- Prompt gates fail closed: modify/ask/defer on a prompt (which the
  hosts cannot apply there) block instead of silently proceeding.
- MODIFY merges: parameter_overrides are per-argument edits, so they are
  merged onto the original input before updatedInput/updated_input
  (which replace wholesale); a modify carrying redactions the host
  cannot express is refused rather than half-applied, and NAT's output
  gate redacts rather than leaking unmodified output on structured
  modify.
- §6.3 composition check implemented: a modifications object combining
  wholesale + structured shapes, or with overlapping JSON-Pointer
  targets, is refused as DENY (shared checker with proper pointer-prefix
  comparison and a false-positive guard test).
- Honesty: profiles_supported advertises acs-core only when signing is
  configured; ACS_HANDSHAKE=0 is audited as non-conformant;
  ACS_DISABLED writes a structured audit event to the durable sink;
  ahead-of-spec behaviors (refusal fail-closed, posture merge,
  ask/defer substitutions) are labeled as deliberate hardening or
  deployment behavior pending the open spec decisions, not cited as
  mandated.

Turn tracking (Claude Code + Cursor):
- A prompt opens a turn with an explicit decision-eligible
  steps/turnStart (a Guardian deny blocks the prompt), the userMessage
  and every in-turn step carry metadata.turn_id, and Stop/stop closes it
  with steps/turnEnd — replacing the wrong Stop->sessionEnd mapping that
  sealed the audit chain after every reply. A stop with no open turn is
  skipped and audited, never a fabricated turn_id. Guardian round-trip
  handling is factored into one shared helper so both POSTs get
  identical binding/signature/refusal treatment.

Cursor host-contract fixes (verified against the official hooks docs):
- Payload builders now read only fields Cursor actually sends:
  workspace_roots (not workspace_path), tool_name/tool_input(+url|command)
  for MCP (not mcp_server/mcp_tool), result_json, text (not
  response/thought), error_message/failure_type (mapped onto the ACS
  exit_status enum incl. timeout and blocked), command/output/duration/
  sandbox for shell results. JSON-stringified inputs are parsed, never
  crashed on. postToolUse's embedded exitCode drives an honest failure
  status; sessionEnd reasons and stop status map onto the ACS enums;
  subagentStart lineage derives from the real tool_call_id;
  beforeSubmitPrompt emits the documented {"continue": false,
  "user_message"} block alongside exit 2; output fields are emitted only
  on postToolUse (the one event that documents them). Emission tests
  feed the documented shapes and assert the content lands in the ACS
  payloads, so schema-valid-but-empty can no longer pass. Claude Code's
  subagent gate matches the current `Agent` tool name (legacy Task kept);
  duration_ms is coerced to the schema's integer.

Docs: mapping.md/README rewritten to match the code (block-shape hooks,
turn model, per-event field sources, correlation gaps stated instead of
papered over); PR GenAI-Security-Project#21 is referenced only as an open proposal not in this
branch; the three normative schema edits carried here are flagged for
explicit spec-owner approval.

Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Bar Kaduri <[email protected]>
@rocklambros

Copy link
Copy Markdown
Contributor

@bar-capsule

This is close. The August commits cleared everything I raised in July and everything @afogel raised in August, and I checked by running the suites instead of reading the diff. claude-code, cursor, and _common all come up green from a clean checkout. NAT's tests need the NVIDIA runtime, so I skipped them.

That last part turned into the one thing I want settled. NAT never got added to the Core suite. Cursor did, back in August, and test_acs_core_conformance.py still doesn't mention nat anywhere. The conformance job only installs requirements-test.txt, so even if you wrote those tests today the job couldn't reach them. Someone reading adapters/README.md comes away thinking all three adapters clear ACS-Core, and two of them do.

Either way NAT ends up in the Core suite. Whether that happens here or in a follow-up is your call. Doing it here means the conformance job installs the NVIDIA runtime as well. If you'd rather push it out, adapters/README.md needs to say what's true in the meantime: the Core suite covers claude-code and cursor, and NAT's conformance rests on adapters/nat/tests/ and the matrix job. What I won't merge is the current version, where the suite says nothing about its own scope and the reader fills in three.

The pins need fixing either way. nat/requirements.txt still floats rfc8785>=0.1,<1 and ruamel.yaml>=0.17, while claude-code and cursor both pin rfc8785==0.1.4, and your own comment in requirements-test.txt lays out why ranges are a bad idea.

adapters/SECURITY.md:228 points researchers at Agent-Control-Standard/ACS. That string is in thirteen files on this branch and zero on main, which means we'd be routing vulnerability reports to a repo we don't own.

The DCO job you added runs on pull_request, and three of your twenty-five commits carry a sign-off. It fails its own branch.

.gitignore is the only merge conflict, so the rebase is cheap.

Please split the schema edits into their own PR. @afogel asked for that in August and never got an answer from me, which I should have handled at the time. It's nine lines across subagent-stop.json, response-envelope.json, and otel-mapping.json, all sitting on ground that #21, #31, and #32 are already arguing over. Split them and this PR stops waiting on a decision it doesn't need.

Ignore what I told you in July about holding for #21. I wanted that sequencing because the suite cited conformance.md by line number, and test_cited_lines_still_carry_their_content handles it better than ordering ever would. Merge whichever one is ready first.

free_port() at _common/test_harness.py:60 closes the socket before it returns, so the port sits free between the return and the Guardian's bind and it'll flake under runner contention. None of the adapters carry a version stamp, which bites later because people install these by copy-paste and there's no way to ask someone what they're running. CONTRIBUTING.md's Local Development section still lists only the mkdocs commands, so nobody learns the suite exists. All three are small enough to fold into whatever round you're already doing.

rocklambros added a commit that referenced this pull request Sep 6, 2026
The site build depends on a bare `uv run pytest -v` running in the
environment uv.lock produces. With no testpaths, pytest collects the whole
repository, so any suite whose dependencies sit outside the lockfile fails at
collection and takes the deploy down with it. PR #22 adds 23 adapter test
files needing rfc8785, nvidia-nat-core, and ruamel, none of them locked:
against a merged tree that is 22 collection errors and an interrupted run,
which fails the test job and stops the build job that depends on it.

Scoping collection to tests/ keeps the deploy gate about the guards it was
written for. Suites carrying their own dependencies run from their own
workflow, which is what adapter_tests.yml already does.

CONTRIBUTING now says where guards live and why a test written elsewhere
never runs, since that silence is what let both contributors build suites CI
would not have executed.

Signed-off-by: rocklambros <[email protected]>
rocklambros added a commit that referenced this pull request Sep 6, 2026
The table mapped ten specific paths and stopped, so a new top-level directory
arrived governed by nothing until someone noticed it was unlisted. The
adapters/ directory in PR #22 is the next one due to land.

Apache 2.0 is the default because a new directory is usually code, and an
over-permissive grant on prose costs less than a ShareAlike obligation
attaching by accident to reference code adopters copy into their own systems.
A prose directory still wants its own explicit CC-BY-SA-4.0 row.

Signed-off-by: rocklambros <[email protected]>
rocklambros added a commit that referenced this pull request Sep 6, 2026
Nobody does, in v0.1.0. profiles_supported and profiles_accepted are
self-declaration on the wire, and the release ships no conformance suite, no
registry, and no steward to arbitrate a disputed claim. The page described
what the label guarantees without saying that the label is the implementer's
own assertion, which is the gap issue #19 raised against a standard that
markets itself as a control standard rather than a wire format.

The paragraph sits below the ACS-Core requirement list rather than inside it,
so the line numbers PR #22's citation guard pins do not move.

Signed-off-by: rocklambros <[email protected]>
@rocklambros

Copy link
Copy Markdown
Contributor

One more thing landed on main, and it's the kind of thing that would have looked like your fault.

deploy-pages.yml runs a bare uv run pytest -v as the gate the site build depends on, and there was no testpaths setting, so pytest collected the whole repository. Your 23 adapter test files need rfc8785, nvidia-nat-core, and ruamel, none of which are in uv.lock. I merged your branch into main locally and collection died with 22 errors, which fails the test job and stops the build job declaring needs: test. The site deploy would have broken on merge with nothing wrong anywhere in your diff.

Fixed in dbbd92d. pyproject.toml now sets testpaths = ["tests"], so the deploy gate stays scoped to the repo's own guards and your suites keep running from adapter_tests.yml exactly as they do now. Nothing for you to change, and your rebase picks it up.

@rocklambros

Copy link
Copy Markdown
Contributor

The skill lifecycle hooks are missing from all three adapters, and the spec page is why

None of the three adapters emit skillRegister, skillLoad, or skillUnload. The methods each one emits today:

Adapter steps/* emitted
claude-code sessionStart, userMessage, turnStart, turnEnd, toolCallRequest, toolCallResult, subagentStart, subagentStop, agentResponse, sessionEnd
cursor same, plus preCompact, minus subagentStop
nat sessionStart, userMessage, toolCallRequest, toolCallResult, agentResponse, sessionEnd

This is our fault, not the contributor's. The hook taxonomy table in Specification section 5 listed sixteen hooks and omitted the entire skill lifecycle set. The Hooks page says nineteen. Both were correct about everything else, so there was no signal that the shorter list was the stale one. Anyone building against the specification page, which is the page that reads as canonical, never learned that skills are a governable surface at all.

I have a fix queued that adds the three hooks to the taxonomy table, renumbers it to nineteen, and adds a guard test that reads the schema titles under specification/v0.1.0/hooks/ and compares them against both readable views plus the count the Hooks page states. The whole drift happened with a green build, so only a check that reads the schemas prevents a repeat.

Worth noting the harness in this PR is already ahead of the spec page: adapters/_common/capture_guardian.py:116-118 maps all three skill methods to their schemas, so validation works the moment anything emits them. Only the emission side is missing.

What this is worth doing about here

Skill emission is SHOULD, not MUST (conformance.md: additional hooks "SHOULD be implemented when the harness can observe the corresponding event"), so none of this blocks the merge. Deferring it to a follow-up is a perfectly reasonable call. Flagging it so the decision is deliberate rather than inherited from a bad table.

For claude-code and cursor there is an observable event: both invoke skills through a tool call, so PreToolUse with tool_name="Skill" is the same shape as the tool_name="Agent" to steps/subagentStart routing the adapter already does. That precedent is the natural place to hang it.

The real obstacle is that skill-load.json requires digest, over the complete loadable artifact. The host gives you a skill name, not a digest. An adapter with filesystem access could resolve the artifact and hash it, but if it cannot resolve it, the honest move is the one already made for PreCompact: document it in KNOWN_UNMAPPED rather than fabricate the field. A fabricated digest is worse than no hook, because skillLoad's entire purpose is binding an activation to an approved (skill_id, digest) registration.

skillUnload needs only skill_id and reason, so it is the cheap one if a partial mapping is attractive.

A gap on our side that would block you

If you do emit skill hooks and want them under ACS-Trace, there is nothing to map them to. The skill lifecycle has no OpenTelemetry span name and no OCSF class, in either the docs tables or the normative mappings:

$ grep -c skill specification/v0.1.0/trace/otel-mapping.json
0
$ grep -c skill specification/v0.1.0/trace/ocsf-mapping.json
0

Same root cause, wider blast radius than the one table. That one is ours to close and I am raising it separately.

One squashed commit carrying the full PR GenAI-Security-Project#22 branch, rebuilt on current
main (DCO: all history signed; branch is a direct child of main).

Adapters (each: acs_adapter, wire.py, mapping.md, README, tests):
- Claude Code: hook-to-steps translation with explicit turn tracking
  (turnStart/turnEnd, per-session state), native allow/deny/ask,
  modify via merged updatedInput, defer substituted to deny + audit.
  SubagentStop is deliberately unmapped: steps/subagentStop requires
  final_chain_hash, which a chain-less framework cannot honestly
  produce; a separate schema PR proposes making it optional.
  ADAPTER_VERSION 0.1.3.
- Cursor: documented-field payload builders (docs.cursor.com,
  2026-08-22), turn tracking, beforeSubmitPrompt exit-2 blocking,
  failure_type-to-exit_status mapping, fail-closed contradictory
  modifications per §6.3.
- NAT: pre/post-invoke middleware with negotiated timeouts, durable
  audit sink, redaction-or-deny output gate.

Shared infrastructure:
- acs_common: RFC 8785 (JCS) + HKDF per-session HMAC signing (§10),
  handshake with signed ServerHello binding and negative cache,
  total decision normalization, §6.3 composition-violation check.
- example_guardian: signed error envelopes, capped regex scanning,
  durable file-locked replay state, subagent gate no weaker than the
  generic tool gate. Binds --port 0 and announces the assigned port
  on stdout so test spawns own their port by construction.
- Conformance + emission suites (230 checks) driving both CLI
  adapters and the Guardian; NAT covered by its own suite (35).

Guardian refusal handling is deliberately stricter than v0.1 spec
text (always fail closed); tracked as spec issue GenAI-Security-Project#32.

Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Bar Kaduri <[email protected]>
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.

4 participants