Skip to content

Let the response envelope carry a ServerHello - #59

Merged
rocklambros merged 2 commits into
GenAI-Security-Project:mainfrom
afogel:fix/response-envelope-handshake-result
Sep 9, 2026
Merged

Let the response envelope carry a ServerHello#59
rocklambros merged 2 commits into
GenAI-Security-Project:mainfrom
afogel:fix/response-envelope-handshake-result

Conversation

@afogel

@afogel afogel commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

What changed

response-envelope.json declared result as an unconditional $ref to AcsResult, which requires decision. handshake.json says a ServerHello is wrapped in that same envelope, and a ServerHello has no decision. So a conformant handshake/hello response could not satisfy the response envelope schema. An implementation that checks its own responses against the schema had to either report a correct handshake as invalid or skip the check for that method.

result is now a oneOf over AcsResult and handshake.json#/$defs/ServerHello. The two shapes share no required field, so a conformant response matches exactly one branch. A decision response that lost its decision still fails, because it matches neither branch.

tests/test_response_envelope.py validates instances against the real package with cross-file refs resolved by $id. It checks that a decision result and a ServerHello are both accepted, that an AcsResult without decision is rejected, and that a ServerHello missing one of its own required fields is rejected.

Type of change

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

Specification changes

  • I opened a Discussion before this PR
  • Schema changes validate against the JSON Schema spec
  • I described the impact on downstream implementers below

Breaking for implementers? No. Every response that validated before still validates. The change admits a shape the prose already required on the wire. Implementers who special-cased handshake/hello to skip response validation can remove that special case.

Checklist

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

Security

  • This change has no security impact

afogel and others added 2 commits September 9, 2026 05:39
response-envelope.json declared `result` as an unconditional $ref to
AcsResult, which requires `decision`. handshake.json wraps a ServerHello
in that same envelope, and a ServerHello has no `decision`. So a
conformant handshake/hello response could not satisfy the response
envelope schema, and an implementation that checks its own responses
had to either report a correct handshake as invalid or skip the check.

`result` is now a oneOf over AcsResult and handshake.json's ServerHello.
The two shapes share no required field, so a conformant response matches
exactly one branch, and a decision response that lost its `decision`
still fails: it matches neither.

tests/test_response_envelope.py validates instances against the real
package with cross-file refs resolved by $id, and holds both branches to
their own required fields.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DN7uYj8dBkUBH2eNccxUpX
Signed-off-by: Ariel <fogeltine@gmail.com>
The response envelope now admits a ServerHello in `result`. Implementers
that validate their own responses will change behaviour on this
distribution, so it needs a release marker they can pin to.

The specification stays at v0.1.0. The change widens the envelope
without invalidating any response that validated before, so the schema
$id, the specification/v0.1.0/ directory, and every $ref stay where
they are. pyproject.toml and uv.lock are synced here by the repo's own
sync_version.py so the Sync Version workflow has nothing to do on merge.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DN7uYj8dBkUBH2eNccxUpX
Signed-off-by: Ariel <fogeltine@gmail.com>

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

The defect is real and the fix is the right shape.

result pointed at AcsResult, which requires decision, while handshake.json puts a ServerHello in that same envelope. A conformant handshake/hello response could not validate against the envelope the prose said carried it. That is a specification bug, not an implementation inconvenience, and it is the kind that only surfaces when someone actually validates their own traffic.

The oneOf discriminates cleanly. AcsResult requires decision, ServerHello requires negotiated_version, and neither shape carries the other's required field, so a conformant response matches exactly one branch. A decision response that lost its decision still fails, because it does not fall through to the ServerHello branch.

The test earns its keep. Building the validator over every $id-bearing file in specification/v0.1.0/ exercises the same cross-file resolution the published package uses, and asserting the envelope $id against BASE in tools/publish_schemas.py ties the test to the real namespace instead of a local copy of it. test_a_server_hello_missing_a_required_field_is_rejected is the one that carries the most weight, because it proves the ServerHello branch is checked against handshake.json rather than waved through.

The non-breaking claim holds. oneOf only widens what validates, so every response that passed before still passes.

I checked the release commit's claim about the Sync Version workflow and it is correct. sync_version.yml guards on git diff --quiet, and pyproject.toml and uv.lock are already synced on this branch, so the workflow will run on merge and find nothing to do.

Landing this as a squash, so it arrives as one commit with your authorship and your sign-off intact. I am dropping the two AI trailers from the commit message. This project keeps human authorship in its git history, and that was written down nowhere when you opened this, which is our gap rather than yours. I am adding it to CONTRIBUTING.md in a follow-up so the next contributor learns it before merge time instead of at it.

Good catch. This is the sort of thing that would have cost an implementer an afternoon.

@rocklambros
rocklambros merged commit 7743826 into GenAI-Security-Project:main Sep 9, 2026
3 checks passed
rocklambros added a commit that referenced this pull request Sep 9, 2026
PR #59 arrived with Co-Authored-By and Claude-Session trailers naming a
model. The trailers were dropped in the squash, which is the right outcome
and the wrong way for a contributor to learn the rule. Nothing in
CONTRIBUTING.md, STYLE.md, or .github/ said it.

The reason is the DCO, which already sits in this file. A sign-off is a
certification a person makes about the origin of the code, and a model
cannot make it. Naming one as a co-author puts a party in the trailer that
cannot stand behind the certification the next line asserts.

The paragraph goes in Development Process, after the spec-Discussion note
and next to the sign-off step it depends on. That keeps it clear of the
adapters hunk PR #22 adds higher in the file.

Signed-off-by: rocklambros <rock@rockcyber.com>
rocklambros added a commit that referenced this pull request Sep 9, 2026
PR #59 fixed a response-envelope defect and shipped the test that proves
the fix. @bar-capsule approved it and the merge stayed blocked, because
CODEOWNERS gave the spec leads /specification/ and /docs/spec/ but not
/tests/, /pyproject.toml, or /uv.lock. A spec lead could approve the schema
change and not the test that holds it to its own contract.

That splits a single reviewable change across two owner tiers, and it will
recur on every spec PR that carries its own coverage. Any fix narrow enough
to unblock only that case would leave the same seam somewhere else, so
@afogel and @bar-capsule join all 21 restricted lines rather than a chosen
subset. Both already hold maintain, so the entries take effect on merge.

The /.github/ comment claimed these paths warrant admin review. Both new
owners hold maintain, so the comment now describes what the list is for and
points at GOVERNANCE.md for who decides membership.

Signed-off-by: rocklambros <rock@rockcyber.com>
rocklambros added a commit to bar-capsule/ACS that referenced this pull request Sep 9, 2026
This branch forked at f46d260 and sat 50 commits behind main. Only the two
version files conflicted. The specification, conformance, and docs changes
merged clean.

version.txt and pyproject.toml moved 0.1.0 to 0.1.1 here. Main has since
shipped 0.1.1 (the Apache 2.0 and CC BY-SA 4.0 relicense marker) and 0.1.2
(the response-envelope ServerHello fix, GenAI-Security-Project#59), so the bump on this branch
now collides with two releases that already exist. The relaxations become
0.1.3, which is the next unused number and gives implementers a marker for
a change to the ACS-Core baseline.

pyproject.toml takes main's file whole. This branch changed only the
version line, and main's copy carries the license field, requires-python
>=3.11, the current dependency format, and the testpaths setting that
scopes pytest collection.

CHANGELOG.md is introduced by this branch, so its history had to be made
true rather than merely renumbered. Its entry claimed 0.1.1 contained the
ACS-Core relaxations, which is not what shipped under that number. The
relaxations entry becomes 0.1.3, and 0.1.1 and 0.1.2 now describe what
those releases actually carried.

Guards pass at 202 and mkdocs build --strict succeeds on the merged tree.

Signed-off-by: rocklambros <rock@rockcyber.com>
rocklambros added a commit to bar-capsule/ACS that referenced this pull request Sep 9, 2026
The merge resolution moved version.txt and pyproject.toml to 0.1.3 and left
uv.lock at the 0.1.2 it inherited from main. The lockfile carries the
project's own version in its acs entry, so the three disagreed.

sync_version.py updates pyproject.toml alone, and the Sync Version workflow
only fires on a push to version.txt on main, so nothing would have caught
this before merge. GenAI-Security-Project#59 kept all three in step and this does the same.

Signed-off-by: rocklambros <rock@rockcyber.com>
rocklambros pushed a commit that referenced this pull request Sep 10, 2026
…gt (#60)

## Summary

Adds `reference-implementations/agt`: a working ACS Guardian built on
Microsoft's [Agent Governance
Toolkit](https://github.com/microsoft/agent-governance-toolkit) (AGT),
two host shims for Claude Code and OpenCode, a shared host adapter, an
envelope Inspector, and a conformance harness that measures what ACS
v0.1.0 can express of AGT, cell by cell. AGT runs unchanged at a pinned
commit. Neither client contains AGT code, and AGT contains no client
code. They share only the ACS envelope.

The tree validates every envelope against this repository's own
`specification/v0.1.0/` schemas, by relative path, so the implementation
and the specification cannot drift apart.

A three-minute captioned demo is embedded in the tree's README: one
Guardian denies the same command from OpenCode and then from Claude
Code, and the conformance harness prints the mapping table, the coverage
matrix and the trace rows.

## What lands

| Path under `reference-implementations/agt/` | What it is |
|---|---|
| `README.md` | How to install, run, read the contract and view the
events. What the tree is and is not, measured against the ACS-Core list.
Links every package README |
| `packages/guardian/` | The ACS server: `POST /acs`, envelope
validation, AGT policy input, verdict mapping, session hash chain,
envelope log. Own README |
| `packages/agt-bridge/` | Constructs the AGT SDK runtime and publishes
the bundled OPA binary to the process environment. Own README |
| `packages/host-adapter/` | Envelope building, handshake, decision
validation, failure posture, audit log. Shared by both shims. Own README
|
| `packages/inspector/` | Tails the Guardian's logs and renders each
envelope, decision, audit entry and chain entry. Own README |
| `packages/conformance/` | The mapping table, the eight-by-five
coverage matrix, the trace rows, and the upstream watch. Own README |
| `hosts/claude-code/`, `hosts/opencode/` | The two shims, their
hookmaps, and their READMEs. The OpenCode README carries a stub model
for a run with no paid account |
| `policy/` | AGT's stock bundle at the pinned commit, byte for byte,
plus `data.json`, two manifests, and the MIT text |
| `mapping.yaml`, `agt.lock`, `bun.lock`, `package.json`,
`tsconfig*.json`, `opencode.json`, `.gitignore` | Configuration and
lockfiles |
| `scripts/` | `verify-pin`, `run-conformance`, `run-upstream-watch`,
`regenerate-curl-resolved-hosts`, `verify-zero-diff` |
| `test/` | The cross-package suite: dispositions, redaction,
information-flow round trip, architecture invariants, the pin, README
captures |
| `docs/demo.mp4`, `docs/demo-poster.jpg` | The demo recording and its
poster frame |

Outside the tree: `LICENSING.md` gets a row and a provenance section for
the vendored MIT bundle, `NOTICE` gets a paragraph, and the root
`README.md` gets one line under Getting Started.

## What was left out, on purpose

The tree was built in
[afogel/ACS_reference_implementation](https://github.com/afogel/ACS_reference_implementation)
over ten slices. The slice records, the shaping documents, the captured
runbooks, that repository's CI workflows, its `SECURITY.md` and its MIT
`LICENSE` are not carried over. They describe how the tree was built,
not how to use it. The README links the standalone repository for that
history.

## Changes made for this location

- `spec/acs` was a git submodule pointing at this repository. It is
gone. `packages/guardian/src/validate-envelope.ts` and
`packages/conformance/src/trace-pillar.ts` resolve
`specification/v0.1.0/` from the repository root by relative path, and
the Guardian's path redaction now strips this repository's root rather
than the tree's. The relocated-copy test in
`packages/guardian/test/server.test.ts` was updated to match.
- Comments that cited the slice runbooks now name the standalone
repository.
- Nothing else in the code changed.

## Measured

From `reference-implementations/agt`, after `bun install
--frozen-lockfile`:

```
bun run typecheck   # zero errors
bun test            # 1109 pass, 1 skip, 1 fail, 68 files
```

The one failure is in `packages/guardian/test/check-response.test.ts`.
It expects `response-envelope.json` to accept a ServerHello as a
`result`, which is what #59 adds. With #59's `response-envelope.json` in
place, the same run is 1110 pass, 1 skip, 0 fail. The skip is the
byte-identity check, which needs `UPSTREAM_BUNDLE` from `bun run
verify:pin`.

So this PR depends on #59. Merge #59 first and the suite is green with
no change here. The README's Verify section states the same dependency
in terms that stay true on any checkout.

## What this is, and is not

It is a working demonstration that AGT's unchanged policy engine can
govern two different agent clients over one wire contract. The tool-call
path is complete in both directions. All five AGT verdicts cross the
wire as ACS decisions. Four of AGT's nine stock gate classes are live.

It is not a complete ACS implementation. The Guardian claims `acs-core`
with qualifications the README lists row by row: two of nineteen hooks
are evaluated, `ask` fails the response schema for want of
`ask_details`, there is no replay protection, no signature, no
`system/ping`, and the wire is unauthenticated. The README's
operational-debt and open-findings sections record the rest.

## Licensing

`policy/lib/` is Microsoft's AGT policy bundle, MIT, byte-identical to
the pinned commit. A test enforces that identity, so the files carry no
added header. `LICENSING.md` gets a row and a provenance section,
`NOTICE` a paragraph, and the MIT text sits at `policy/LICENSE-AGT`.
Everything else in the tree lands under Apache 2.0 through the existing
catch-all row.

## Not in this PR

- No CI workflow. The suite needs bun and `trash`, so under CONTRIBUTING
it belongs in its own workflow. I can add one scoped to
`reference-implementations/agt/**` if you want it in this PR or a
follow-up.
- #22 proposes `adapters/` for configuration-only host adapters. This
tree's subject is the Guardian side and its measurement of the contract,
with the two host shims as the clients that prove it. If you prefer one
directory for both, the tree can move.

---------

Signed-off-by: Ariel Fogel <fogeltine@gmail.com>
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.

3 participants