Skip to content

docs(instrument): document a MODIFY tool-call exchange - #271

Open
Lrd0036 wants to merge 3 commits into
GenAI-Security-Project:integrationfrom
Lrd0036:lrd/docs-acs-instrumentable
Open

Lrd0036 wants to merge 3 commits into
GenAI-Security-Project:integrationfrom
Lrd0036:lrd/docs-acs-instrumentable

Conversation

@Lrd0036

@Lrd0036 Lrd0036 commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

The first slice of #267. In acs.md's Instrumentable section, the MCP tab is replaced with a steps/toolCallRequest/MODIFY exchange, and the A2A and SDK tabs are removed. A new guard validates every message in the section against the v0.1.0 schemas and checks the required Core fields those schemas leave optional.

Which issue does this implement

Part of #267

Changes

New "ACS wire (JSON-RPC)" tab. It keeps the old tab's scenario: an agent asks to run SELECT * FROM customers, and the Guardian narrows it. The request and response now use the defined shapes.

  • Request: steps/toolCallRequest with the required acs_version, request_id, timestamp, and metadata; a payload with tool.name and a wrapped query argument; and the HMAC-SHA256 signature ACS-Core requires (§10).
  • Response: a final result with decision: "modify", the required reasoning, modifications.parameter_overrides, and the chain_hash §8.6 requires on a content-bearing step.
  • Notes under the exchange: remove params.signature or result.signature before JCS canonicalization and UTF-8 encoding (§10). Signature values are placeholders, MODIFY is SHOULD-support with a deny substitution (§6.5), an MCP tools/call can use the same hook, and A2A is reserved for v0.2.

A2A tab removed. It listed a hook table, including a message hook, that no specification defines. v0.1 has no A2A wire to show in its place.

SDK tabs removed. The LangChain and Vercel tabs imported acs and @acs/guardian, which ACS does not publish. Following the November 1 ruling on #267, they come out with no "illustrative" label.

tests/test_acs_examples.py. Envelope validation alone would pass steps/anything with any payload, so the guard checks more than the envelope:

  • A request must name a method that has a payload schema, fit the request envelope, satisfy its payload schema, and carry its Core signature. system/ping retains its signature exemption.
  • A response must fit the response envelope, answer a request shown beside it, carry its Core signature and the toolCallRequest chain head, and override only arguments that request sent.
  • Every request is validated before indexing by ID. Duplicate request IDs in the section are rejected so one request cannot hide another or make response pairing ambiguous.
  • Mutation tests reject the old methods, an undefined steps/registerHook, a bare argument value, and malformed MODIFY results. New regressions reject unsigned Core requests and responses, missing chain heads, and invalid requests sharing an ID with a valid request in either order. A separate case rejects duplicate valid request IDs.

Other sections join the guard's SECTIONS list as their slices land; the content-bearing response checks must expand with them.

Judgment calls for review:

  • Signature values are placeholders. They describe HMAC-SHA256 over UTF-8 JCS of the whole envelope with the relevant signature object removed. Since this PR opened, Fix the HMAC key derivation, define key_id, and record an unresolved request_id_ref #272 pinned the HMAC key derivation (HKDF-SHA256, no salt, info set to metadata.session_id) and defined key_id as an opaque label the deployment chooses, so real values are now computable from a published sample secret. This slice keeps placeholders; switching to real values that the guard verifies can follow if reviewers want it. The guard checks structure and required field presence; it does not verify the illustrative signatures or sample chain head.
  • parameter_overrides uses a bare replacement value, matching the schema's description ("replacement values for tool call arguments, keyed by argument name") and how the reference host adapter applies it.

Base branch

  • integration, because this touches tests.

Type of change

  • Documentation
  • Tooling or CI (documentation guard in tests/)

I tested this

  • The branch merges the current integration commit, bba1b84 (checked 2026-11-04).
  • uv run --locked pytest -q passes on my machine: 1222 passed, 1 skipped, including all 17 tests in the new guard
  • uv run --locked mkdocs build --strict passes on my machine
  • git diff --check passes

The strict build doesn't validate anchors, so I checked the four new link fragments against the built HTML (rechecked 2026-11-04). The tab renders both JSON blocks as highlighted code. Separate page-level mutation probes confirmed that removing each required field, inserting a shadowed invalid request, or repeating a valid request ID fails on the intended rule.

An isolated merge with #247 at 147f7d4 has no conflicts. The full pytest suite passes on that combined tree (1237 passed, 1 skipped), including all 32 tests across the two example guards. #247 remains open on GitHub; this is local combined validation.

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

  • No material security impact identified

No material runtime change identified. The documentation explains existing ACS-Core signing and chain-head requirements; the schemas and signing implementation are unchanged. Placeholder signatures and the sample chain head demonstrate message structure, not authenticated interoperability.

Architecture and knowledge

Only the overview documentation and its guard change. The normative signing contract in §10 and chain-head rule in §8.6 remain the authoritative sources.

Follow-up

The remaining #267 pillars stay open, including the SDK tabs in the other sections. The OCSF tab and Inspect extension mappings are outside this slice. Revisit the ping-response signature interpretation when a ping example is added; this slice contains only a tool-call exchange.

Part of GenAI-Security-Project#267.

The Instrumentable tab used undefined ACS methods and an invalid MODIFY
response. Replace it with the database_query toolCallRequest/modify scenario
using the v0.1 envelopes and payload, and remove the v0.2 A2A tab. Leave the
SDK tabs pending the removal or illustrative-label choice on GenAI-Security-Project#267.

Document the existing ACS-Core signing input: UTF-8 JCS of the whole
envelope after removing params.signature or result.signature (§10). Keep
explicit placeholders for signatures and a sample chain head; no
cryptographic authenticity or authenticated interoperability is claimed.

Guard every displayed request before indexing by ID and reject ambiguous
duplicate IDs. Check defined methods, payloads, response pairing, argument
overrides, Core signature presence, and the toolCallRequest chain head (§8.6).
The schema checks alone leave those required Core fields optional.

Regression cases reject omitted signatures and chain heads, invalid
requests sharing an ID with valid ones, and duplicate valid request IDs.

Signed-off-by: Lrd0036 <lrd0036@auburn.edu>
@rocklambros

rocklambros commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

@Lrd0036, since you already have this moving, can you run with the rest of issue #267 in this PR? I've taken myself off the issue, so it's yours.

That covers everything on the checklist at the top of the issue that lives in this repo: every SDK tab in docs/acs.md, the acs-sdk-1.0 line, the wire, A2A, and Inspect examples, landing/index.html:115, docs/spec/instrument/README.md:13, and the guard in tests/. tests/test_acs_examples.py looks like the natural home for that guard.

One thing sits outside this PR, so please keep Part of #267 in the description. The OCSF tab goes with your PR #247.

If the PR gets too big to review in one pass, tell me and we'll split it.

@drk-ram drk-ram 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.

Reviewed at head 92c3128105e323febd68e50f40ab8606b6f2e16b. Approving this slice of #267. The new example is faithful to the normative text, and the guard makes the failure that produced the old example much harder to repeat.

Checked against the specification

  • Every wire element in the new "ACS wire (JSON-RPC)" tab matches the v0.1.0 schemas. request-envelope.json and response-envelope.json accept both messages, and the payload satisfies hooks/tool-call-request.json (tool.name, with each argument wrapped as { "value": ... }).
  • The signing note matches §10. The signed input is the RFC 8785 (JCS) canonicalization of the envelope with signature removed, encoded as UTF-8. Naming params.signature for requests and result.signature for responses is the right JSON-RPC reading of that rule.
  • The chain_hash on the modify response matches §8.6, which requires it on content-bearing steps and under the response signature.
  • The MODIFY-incapable client note matches §6.5. The substituted deny with reason_codes: ["modify_unsupported"] is the specified behaviour.
  • Deferring A2A to v0.2 is the honest choice, and removing the old A2A tab is correct. It documented a hook table, including a message hook, that no part of the specification defines.

Validation I ran on a clean checkout of this head (Windows host)

  • uv run --locked pytest -q tests/test_acs_examples.py: 17 passed.
  • uv run --locked mkdocs build --strict: passed.
  • Full suite, excluding tests/test_closing_choice.py, which errors on this host: 866 passed, 10 failed. The failures are in tests/test_fetch_roadmap.py (3), tests/test_pr_intake_workflow.py (5) and tests/test_publish_schemas.py (2). None of those files are touched by this PR. They depend on symlinks and shell workflows that my host handles differently, so I am treating them as environmental, not as regressions.
  • Merge simulation: this branch merges without conflicts with #247 (cffa5a870dd8) and with #253 (f033a6365fc9), which also edit docs/acs.md and docs/spec/instrument/hooks.md.

On the guard. Validating only the envelope would accept steps/anything with any payload. Resolving each method to its payload schema, requiring request/response pairing, rejecting duplicate request IDs and requiring the Core signature and chain_hash fields all close real holes. The mutation cases (old method names, bare argument values, unsigned messages, shadowed and duplicate IDs) are the right set.

Non-blocking observations

  1. parameter_overrides carries bare replacement values, while the request's arguments are wrapped objects with value and optional provenance. The bare form is what the schema and the reference adapter use, and your note says so. A reader still sees an asymmetry. One sentence saying that an overridden argument is a replacement value, and what provenance, if any, it carries under provenance_producer: deterministic, would pre-empt the question.
  2. The sample chain_hash is a 64-hex string and the note says it is a sample. A visibly synthetic value would stop anyone copying it into a verifier test. This also applies to the placeholder signature text.
  3. In the guard, section() ends a section at the next \n#{1,3} line, and messages() matches only fences tagged exactly json. A future jsonc fence or an unindented # line inside a code block would be skipped or truncate the section without failing. A short comment on that assumption, or an assertion that every fenced block in a guarded section is parsed, would make it robust as more sections join SECTIONS.
  4. Since #272 pinned the HKDF derivation, computing real signature values from a published sample secret and having the guard verify them would turn the guard into an interoperability fixture. A later slice is fine.

@rocklambros asked for the rest of #267 to land in this PR. My approval covers the content at 92c3128105e3; any later commits that expand scope will need their own look from me.

Signed-off-by: Lrd0036 <lrd0036@auburn.edu>
The November 1 ruling on GenAI-Security-Project#267 removes SDK code from the docs, because
ACS publishes no SDK or package. The LangChain and Vercel tabs imported
acs and @acs/guardian, so they come out with no "illustrative" label.
The section now shows only the ACS wire exchange the guard checks.

Signed-off-by: Lrd0036 <lrd0036@auburn.edu>
@Lrd0036

Lrd0036 commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

@rocklambros the two SDK tabs are out of the Instrumentable section (de524c6), with no "illustrative" label, and the description no longer says they stay. I also merged current integration, and the full suite passes.

@drk-ram drk-ram 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.

Reviewed at head de524c6296be364d88f70350cbcdc0b3cc3ffc2c. Approving. My earlier approval at 92c3128 covered the content then; this review covers what has changed since.

What changed since 92c3128. A merge of integration at bba1b84, and de524c6, which is 71 deletions in docs/acs.md and nothing else: the "LangChain (Python)" and "Vercel AI (TypeScript)" tabs come out of the Instrumentable section. I read the commit on its own. It does not touch the guard, the wire tab, or any other section, so the 17 tests and the exchange I approved before are byte-identical.

What I checked

  • The two removed tabs imported acs and @acs/guardian, packages ACS does not publish. They go with no "illustrative" label, which is what the 1 November ruling on #267 asks for, and the description no longer says they stay. The section now shows only the exchange the guard checks.
  • The tab group in that section now has a single tab, "ACS wire (JSON-RPC)". It renders under mkdocs build --strict, and no prose in the section refers to the removed tabs.
  • The remaining SDK imports on docs/acs.md (the quick-start tabs near the top, and the Traceable and Inspectable sections) are outside this slice. The description keeps Part of #267 and says so under "Follow-up", so I am not asking for them here.

Validation at this head (Windows host)

  • uv run pytest -q tests/test_acs_examples.py: 17 passed.
  • Full uv run pytest -q: 1202 passed, 1 skipped, 10 failed, 20 errors. The 30 are in test_closing_choice.py, test_fetch_roadmap.py, test_overrides.py, test_pr_intake_workflow.py and test_publish_schemas.py; the same five files fail identically on integration at bba1b84, and none is touched here.
  • uv run --no-dev mkdocs build --strict -d _site/docs: exit 0.
  • Merge simulation against #247 (147f7d4), #498 (d3966f3) and #496 (034f52a): no conflicts.

Non-blocking, and new. I added an unknown key ("bogus": 1) beside capability in the request payload and all 17 tests still passed. That is how the payload schemas are written (they are extensible), but it means a misspelled field in the page example, such as capabilty, would not be caught. #247's guard has a declared_objects_only helper that closes this for its example by setting additionalProperties: false on declared objects. Reusing it for this payload and the response result would make the guard catch a typo, not only a wrong method or a missing field. It can wait for the slice that adds the next section to SECTIONS.

My four non-blocking notes from the earlier review are unchanged and still optional. Approving.

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

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

3 participants