Skip to content

Say a dry-run action was not enforced only when the policy refused it and it went ahead - #530

Merged
davidmckayv merged 2 commits into
CopilotKit:mainfrom
zopeVaibhav:fix/audit-dry-run-label
Sep 14, 2026
Merged

davidmckayv merged 2 commits into
CopilotKit:mainfrom
zopeVaibhav:fix/audit-dry-run-label

Conversation

@zopeVaibhav

Copy link
Copy Markdown
Contributor

Fixes #529

What this changes

On a deployment whose policy is in dry-run, the Audit page printed "dry-run: recorded, not enforced" under rows that were never refused and under a tool call that was actually stopped. The line is meant for one case: the policy refused an action and dry-run let it through. It was drawn on decision.mode === "dry-run" && decision.carriedOut, and an allowed action is always carried out, so every allowed row on a dry-run deployment qualified.

Two small changes, one on each side.

The page now also requires decision.allowed === false before drawing the line. That is the whole definition of a refusal dry-run did not enforce, and it reads rows already in the trail correctly, including content-inspection refusals written before this change, because those carry allowed: true from the policy step.

The content-inspection refusal row in server/src/plugins/store.ts now records decision.carriedOut: false. It was built by spreading the policy step's decided object, which already held carriedOut: verdict.forward, so a call inspection refused was recorded as carried out in every mode, enforce included. The changelog entry that introduced the field defines it as what tells "a call this deployment stopped from one dry-run recorded and let past", and this row now agrees with that.

Deliberately left alone: the page still shows no reason line for a content-inspection refusal (it prints payload.reason for mcp.call_rejected, and this row stores refusal instead), and a call that a dry-run deny rule matches and that inspection then stops still writes two rows, the first of which correctly says the policy's refusal was not enforced. Both are separate questions about what the page should say, not about this label.

Where it runs

  • New state that outlives a request? None. One boolean in an audit row's payload and one condition in a component.
  • What happens on the second replica? The same as on the first. Every server process writes the same payload for the same refusal, and the page renders from the row alone.
  • Anything serialised? No.
  • Anything fanned out to a browser? No.
  • New listener, port, or schedule? No.

Boundary and audit

The gateway order is unchanged, and nothing about what is refused changes. The content-inspection refusal still writes mcp.call_rejected before it throws; only the value of decision.carriedOut in that row differs. Nothing in server, worker or app reads carriedOut except this page, and rows written before the change are not rewritten.

Changelog

A line under Unreleased describing the label and the corrected field.

Proof

  • New app/tests/audit-dry-run-label.test.tsx renders the real Audit route with four rows. Against main (a97f775) in a separate worktree, the two bug cases fail (an allowed dry-run action, and a content-inspection refusal under dry-run, both showing the line) and the two controls pass (a dry-run refusal that went ahead shows the line, an enforce refusal does not). On this branch all four pass.
  • server/tests/plugin-store.integration.test.ts "credential material is refused and never copied into the audit trail" now also asserts decision: { carriedOut: false }. Against main it fails with "carriedOut": true; on this branch the file is 62 pass, 0 fail.
  • Rows written by the real plugin store under { mode: "dry-run", deny: [], allow: ["true"] } on a migrated test database: an allowed call records allowed: true, carriedOut: true, and a call carrying an apiKey records mcp.call_rejected with carriedOut: true on main and carriedOut: false on this branch.
  • bun test app/: 771 pass, 0 fail on this branch across 89 files, and 767 pass, 0 fail on main across 88 files in the same session. The difference is the four new tests.
  • bun test server/ with TEST_DATABASE_URL pointing at a dedicated test database: 2449 pass, 0 fail on both, with identical failure lines (none).
  • bun run typecheck, bun run lint and bun run format:check are clean.

@zopeVaibhav
zopeVaibhav force-pushed the fix/audit-dry-run-label branch 3 times, most recently from 4ab50b1 to 3364875 Compare September 14, 2026 16:40
@zopeVaibhav
zopeVaibhav force-pushed the fix/audit-dry-run-label branch from 3364875 to 4625f77 Compare September 14, 2026 16:45

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

Deep-reviewed clean (validation, no secret leak, fail-closed, agrees with existing layers). CI green.

@davidmckayv
davidmckayv merged commit c34ebd1 into CopilotKit:main Sep 14, 2026
15 checks passed
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.

Every allowed action on a dry-run deployment's Audit page says it was not enforced

2 participants