Repository navigation
Conversation
acs-board-reconciler
Bot
moved this from Needs triage
to In progress
in ACS Project Tracker
|
Thanks for the Codex slice. I found one interoperability issue in The current code reads server_hello = (response.get("result") or {}).get("payload")ACS v0.1 returns server_hello = response.get("result")The TypeScript Guardian uses this standard handshake shape too. Could this be corrected and covered by an interoperability test against the TypeScript Guardian? |
|
Yes! Thanks for catching this. d077978 fixes both fresh and cached reads, both Python response producers, and the affected fixtures. It adds 13 regressions while preserving signature verification, request binding, and whole-envelope caching. The repaired local adapter gate passed 304 tests with three expected live-product skips; the PR description now separates those results from the original commit's results. The TypeScript portion is still open. I ran the TypeScript Guardian, and its direct ServerHello validates against the handshake schema... but the response is unsigned. Its README documents the authentication/signing bit. With a nonempty key and signature verification enabled, Codex rejects that peer. This is wire-shape compatibility and rejection coverage, not a successful signed integration, and the diagnostic is not committed in this repair. Would a committed test that launches the TypeScript Guardian, validates its direct-result shape, and asserts rejection by the signed Python/Codex path meet the coverage expected for this repair, with positive signed end-to-end interoperability tracked separately pending TypeScript signing support? The new head's Deploy Pages and adapter-tests runs both show Follow-up: I’ve committed the TypeScript coverage in b2d4b49. It launches the actual Guardian, validates the direct ServerHello shape, and checks that the shared helper and default-deny Codex adapter reject its unsigned response. Signature verification remains enabled. |
|
@Lrd0036 you had the ServerHello fix up the same day @President found it, with 13 regression tests behind it, and I've got one housekeeping ask. Could you add |
|
Done, thanks! |
|
Oops. Both authoritative and conformance failures come from the same citation guard in adapters/test_acs_core_conformance.py: an upstream diagram moved the specification’s line numbers.
|
|
Fixed & pushed 49f5232. Citation guard now follows ACS-core reqs. instead of the fixed line numbers. Also addressed that workflow cleanup flagged by #179. |
…179) CodeQL default setup was turned on for this repository on 2026-09-21 and covers the Actions language. This adds zizmor, which the [OWASP GitHub Actions Security Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/GitHub_Actions_Security_Cheat_Sheet.html) recommends beside CodeQL for defense in depth. zizmor reads each workflow's logic and checks what CodeQL does not: hash pins no tag points to, version comments that disagree with their pin, App tokens broader than the job needs, and credentials a checkout leaves in `.git/config`. Run against `integration` before this change with online audits on, zizmor 1.30.1 reported 15 findings. This clears 13. The other two are `github-app` findings on `board-reconcile.yml`, whose App token takes the installation's full grant instead of naming its repository and permissions. Token scopes are not changing right now, so those two stay open in code scanning, where they record the deferred decision instead of disappearing behind a suppression. - **`pr-intake.yml`, 1 high.** zizmor flags every `pull_request_target`. This one checks out no code and reads the pull request body as data through an environment variable, which the workflow's header already explains. It carries an inline `zizmor: ignore` that points at that explanation. - **Eleven checkouts across nine workflows, 11 low.** Each left the job's token in `.git/config`. None of those jobs pushes with git. They call `gh` with `GH_TOKEN` or touch no network at all, and `sync_version.yml` hands its push to `create-pull-request`, which configures its own token. All eleven now set `persist-credentials: false`. The checkout in `validate-owasp-metadata.yaml` also gains the `# v7.0.1` comment every other pin carries. - **`reference-implementation.yml`, 1 medium.** The setup-bun pin `735343b` is v2.0.2, and its comment said v2.0.1. Only the comment changes. Dependabot's #83 already treats it as 2.0.2. The new workflow runs zizmor from a `zizmor` dependency group in `uv.lock`, pinned with hashes. No new action enters the Actions allowlist, and Dependabot's uv ecosystem moves zizmor under the existing 7-day cooldown. The group is not a default group, so `uv sync --locked` in the deploy and test jobs never installs it. A default sync against this lockfile would uninstall it. It runs on pull requests and pushes that touch `.github/`, an action definition, `pyproject.toml`, or `uv.lock`, and weekly, because the online audits compare pins against advisories published after the pin. Findings upload as SARIF under the category `zizmor`, and the job stays green on findings so code scanning tracks them and a ruleset can gate on them later. A tool or upload failure still fails the job. A pull request from a fork cannot upload with its read-only token, so it gets annotations instead. `upload-sarif` is pinned at v4.38.0 because v4.38.1 was three days old, inside the cooldown this repository applies to its dependencies. Open pull requests #162 and #169 add workflows that zizmor will flag once they land, for unset `persist-credentials` and the same `# v2.0.1` setup-bun comment. Verified locally: zizmor 1.30.1 with online audits reports only the two deferred `github-app` findings, with 1 ignored and 26 low-confidence findings suppressed by the default persona. The workflow's own command produces valid SARIF 2.1.0. `actionlint` is clean. `uv lock --check` passes under uv 0.9.9, the version CI pins, and the lockfile diff adds only zizmor. `uv run pytest` gives 278 passed, 1 skipped. Signed-off-by: Rock Lambros <rock@rockcyber.com>
|
I reran |
Thanks for the rerun + identifying the project-file and trust gap. I followed up locally on 49f5232 with Codex CLI 0.156.1. In a disposavle project, Codex discovered the hook from .codex/hooks.json... I reviewed and trusted it. Separate ALLOW, DENY, and MODIFY runs then worked without the inline hook configuration or --dangerously-bypass-hook-trust: only the original command ran for ALLOW, neither ran for DENY, and only the replacement ran for MODIFY. Each trusted run sent a handshake and a schema- and signature-validated tool-call request. Also ran DENY before trusting the hook. Codex sent no Guardian request and ran the original command. That gives us a live example of the missing-hook ambiguity @rocklambros and I discussed in #156: a Guardian record alone cannot tell us what happened when the Guardian never got the info. No PR files changed. But this is evidence for project-file discovery and saved trust on the exec_command path. |
rocklambros
changed the title
rocklambros
changed the title
Port the Codex adapter from GenAI-Security-Project#162 onto the package layout GenAI-Security-Project#22 lands, as acs.integrations.codex with an acs-codex-hook command. The adapter now goes through acs.client for the handshake, signing, the round trip and the fail posture, so it follows the same §4.1, §6.4 and §10 rules as the other hosts, the key_id and 32-byte secret included. What stays Codex's own: an event without its session, turn or tool-call id, or with a non-object input, is denied with no traffic. ASK and DEFER deny, since Codex fails a PreToolUse ask and runs the call. A MODIFY is applied only as parameter_overrides, and only as a command override on Bash and apply_patch. A negotiated decision timeout over 10 s denies before the step is sent. Two behaviors change from GenAI-Security-Project#162. An ALLOW prints nothing, so Codex's own approval policy and sandbox still decide, as GenAI-Security-Project#22 does for Claude Code and Cursor; the Codex hook docs do not say whether an explicit allow skips approval. A ServerHello that fails its checks, an unreadable response and a dependency that fails to import now follow the shared posture rules instead of always denying. The live check passed against Codex CLI 0.142.5 on macOS: ALLOW ran the original command, DENY ran neither, MODIFY ran only the replacement. Signed-off-by: Lrd0036 <lrd0036@auburn.edu>
…adapter's tests and docs GenAI-Security-Project#22 now denies by default: a session whose handshake gets no answer denies unless the hook sets ACS_STARTUP_POSTURE=proceed (741d151), and a call with no usable decision denies unless the hook sets ACS_DEFAULT_DENY=0 (b9e1f5e). The Codex adapter already uses the shared config, so only its tests and README still described the old defaults. The two posture tests now check each combination: an unanswered handshake proceeds only under ACS_STARTUP_POSTURE=proceed, and a hook whose adapter cannot load proceeds only when both postures opt in. The README rows for ACS_DEFAULT_DENY and ACS_STARTUP_POSTURE match. Signed-off-by: Lrd0036 <lrd0036@auburn.edu>
Codex 0.142.5 runs a call after any PreToolUse failure except exit 2, so a missing or broken acs-codex-hook let every call through. The example now runs the hook under sh -c with an exit-2 fallback. Codex also ignores continue: false from SubagentStart, and multi_agent is on by default, so the ^Bash$ matcher left spawns ungated. The example now matches spawn_agent too, and the Guardian sees a spawn as a steps/toolCallRequest. The live check runs under the example's matcher and fallback, and it adds spawn ALLOW, DENY and MODIFY cases and a hook-cannot-run case. All seven pass on Codex CLI 0.142.5. A unit test pins the example's matcher and fallback. Signed-off-by: Lrd0036 <lrd0036@auburn.edu>
5b5bfac to
0771f46
Compare
|
This pull request's description says it closes #170, which is on the project roadmap. When it merges, GitHub closes that issue and the roadmap counts it as delivered. Please confirm the change delivers the whole issue before merging. |
…gents Codex offers apply_patch to every catalog model, and the hook receives it as tool_name apply_patch with the patch in tool_input.command. The example matcher left it out, so a file write the Guardian denied as a shell command could go through as a patch. The matcher now includes apply_patch. tool_use_id is the model's own call ID, so a later turn can reuse it, and a subagent runs under the parent's session in a turn of its own. The request ID now includes turn_id, so a reused call ID is not taken for a replay. A subagent's steps now carry Codex's agent_id and agent_type as metadata.codex_agent_id and metadata.codex_agent_type. The example drops env: sh sets the ACS_* variables itself. mapping.md records the MODIFY result from the interactive CLI: under -a untrusted, Codex still asks before a rewritten command runs, and the prompt shows the rewritten command. The live check adds apply_patch ALLOW, DENY and MODIFY cases and checks that a subagent's Bash call names the subagent. All 10 cases pass on Codex CLI 0.142.5. Signed-off-by: Lrd0036 <lrd0036@auburn.edu>
What changed
An ACS Guardian can now allow, deny or rewrite a Codex shell command or subagent spawn before it runs. Codex runs
acs-codex-hookas itsPreToolUsehook, which sends the call as a signedsteps/toolCallRequestand returns the decision as Codex's own hook output.The adapter is part of #22's
acspackage, atadapters/src/acs/integrations/codex/, and uses its signing, handshake and posture code. This push replaces the earlier branch, which was built on #22's old flat layout. It is three commits onintegration:4ce2e59acs-codex-hookcommand, its tests, docs and live check, and registers it in the conformance runner, CI and the wheel check27ee37fACS_DEFAULT_DENYandACS_STARTUP_POSTUREdeny unless set to opt out0771f46What the last commit fixes
Live testing on Codex CLI 0.142.5 found two ways the earlier example configuration failed open:
PreToolUsefailure except exit 2. A missing binary, a crash or invalid output all count. The example now wraps the hook insh -c '<hook> || { …; exit 2; }', which blocks instead. The adapter exits 0 on every path, so the fallback fires only when the hook cannot run.^Bash$matcher missed them.continue: falsefromSubagentStartdoes not stop a spawn, but aPreToolUsedeny on thespawn_agenttool does. The matcher is now^(Bash|spawn_agent)$, and the Guardian sees a spawn as a tool call namedspawn_agent.What happened to the earlier repair commits
d077978(ServerHello shape) and49f5232(citation guard, CI credentials): Add adapters/ with Claude Code, Cursor, and NAT reference implementations #22 already does both.5b5bfac(unevaluated methods): carried forward, with its tests.b2d4b49(TypeScript Guardian interop tests): not carried. Add adapters/ with Claude Code, Cursor, and NAT reference implementations #22's package has no interop suite. Those tests only showed that the adapter rejects the TypeScript Guardian's unsigned reply.Which issue does this implement
Closes #170
Part of #89
meaning, so it needs no issue
Base branch
integration, because this touches the specification, schemas, a referenceimplementation, an adapter, tests, or CI
main, because every changed path is on the documentation lane allowlist inCONTRIBUTING.md
Type of change
codexto the adapter test commandI tested this
uv run pytest -vpasses on my machine: 1205 passed, 1 skippeduv run mkdocs build --strictpasses on my machineAlso run locally on
0771f46:run_conformance.py claude codex cursor nat: 687 passed, 3 expected skipslint-imports: 2 contracts keptcheck_wheel.pyon the built wheel, Python 3.11 and 3.12: no failed checkslive_verify.pyagainst the real Codex CLI 0.142.5, under the example's matcher and wrapper: all 7 cases passedBashcallThe live check uses a scripted model on localhost, so it tests enforcement, not model judgment. It passes hook trust with Codex's one-run bypass flag.
Checklist
git commit -s(required by the DCO)Security
This adapter decides whether Codex tool calls run. It denies on missing signing material, invalid input, a bad or mis-bound signature, a refusal code, and any MODIFY it cannot apply in full. ASK and DEFER become audited denies, because Codex does not support
askand runs the call anyway.Codex still fails open in places this adapter cannot fix. The README lists them:
send_inputandresume_agentare not matched. Either can give a subagent new work.agent_id, so the Guardian cannot tell them apart.PreToolUse.