ACE-035: shared guardrail contract — Verdict / policy / Envelope by sandeep-agami · Pull Request #111 · AgamiAI/agami-core

sandeep-agami · 2026-07-12T01:17:38Z

Summary

Introduces one shared result shape for the SQL-execution guardrails and refactors the safety gates + the execute_sql result wire onto it. Every gate now produces a Verdict; one policy(verdict, tier) maps a verdict to an action by class + deployment tier; and every surface (stdio + HTTP MCP) returns one Envelope. The ad-hoc gate return shapes and the hand-rolled error dicts are deleted, not wrapped. This is the root of the safe-SQL-execution work — the data-protection and governance gates build to these same types in their own changes.

Changes

New guardrail.py (stdlib-only, vendored): Verdict (a gate's finding), Refusal, Envelope (the one result shape), and policy(verdict, tier) -> action. The safety branch is fully implemented (reject in every tier; fail-closed on uncertainty); the data-protection and governance branches are documented stubs for later work.
Safety gates return Verdict: sql_guard.check_read_only + runtime.{check_table_scope, check_no_select_star, check_column_scope} now return Verdict | None (None = allow); the three ad-hoc *Result dataclasses are deleted.
One Envelope on both surfaces: the executor emits a {"refusal": …} line; tools.tool_execute_sql assembles the Envelope — a safety violation → status=refused + refusal{kind,reason,remediation} + no data; a clean query → status=ok + data (the existing ExecuteSqlResult payload) + audit_id. ErrorResult/ToolError folded into Refusal.
Audit trail: new portable guardrail_audit table (migration 012); every ok/refused result writes one row keyed by audit_id at the shared chokepoint (so both surfaces are covered), best-effort.
Governance seam: the old GovernanceVerdict result type is retired; the GovernancePolicy injection seam is kept, re-typed to return list[Verdict] (a no-op default for later governance work).

Design decisions

Kept the governance plug-point (re-typed to Verdict) rather than deleting it — later governance work injects through it.
Envelope is the single top-level result; the well-modeled ExecuteSqlResult is reused as its data payload (not flattened). Pre-release, so no compatibility shims.
Refusal.kind is an open str documented by REFUSAL_KINDS — operational failures (syntax/auth/driver_missing) come from the DB driver, not a fixed guardrail vocabulary — with a test pinning the emit sites against the documented set.
The audit trail is a dedicated table written at the executor chokepoint: the generic tool-call log is written only by the HTTP transport, so it would miss the stdio surface.

Test plan

Full gate green: uv run dev.py check — 1494 tests, ruff, format, gitleaks, and vendored-lib drift.
New tests: test_guardrail.py (types + policy), test_guardrail_audit.py (audit trail), test_execute_sql_envelope.py (the executor→tool refusal relay), and tests/e2e/test_safety_envelope.py (one Envelope over the real stdio + HTTP transports).
~78 existing gate/wire assertions migrated to the new shape without weakening (refusals still assert refusal; offending names still checked).

Checklist

Full gate green (lint, format, tests, secret scan, vendored-lib drift)
Both surfaces (stdio + HTTP) return the Envelope and are tested on both
No second SQL parser; the single execute_sql enforcement point is preserved
Adversarial safety tests pass; fail-closed behavior preserved
Reviewed (code-review + security-review panel); findings dispositioned (raw operational error-text sanitization is a separate change)

Spec: ACE-035

🤖 Generated with Claude Code

Copilot

Pull request overview

This PR introduces a shared, stdlib-only “guardrail contract” (Verdict → policy() → Envelope) and refactors SQL execution guardrails plus the execute_sql tool wire format (stdio + HTTP MCP) to consistently return a single response shape. It also adds an audit trail written at the shared tool chokepoint to ensure both transports are covered.

Changes:

Adds guardrail.py (vendored stdlib-only) defining Verdict, Refusal, Envelope, and policy(), and updates safety gates to return Verdict | None.
Refactors execute_sql/tool_execute_sql to emit/assemble the unified Envelope (status=ok|refused, refusal, data, audit_id, etc.) and updates tests accordingly.
Adds guardrail_audit persistence (migration + sink method) and tests validating best-effort audit writes (DB + jsonl fallback).

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/test_table_scope_gate.py	Updates assertions for table-scope gate to the new `Verdict
tests/test_sql_guard.py	Migrates `check_read_only` tests from reason strings to `Verdict.detail` and updates refusal envelope expectations.
tests/test_ports.py	Updates governance seam expectations to `list[Verdict]` (OSS default empty list).
tests/test_plugin_lib_resolution.py	Ensures `guardrail.py` is included in vendored plugin lib set.
tests/test_mcp_harness.py	Updates MCP harness assertions to the `Envelope` refusal shape.
tests/test_guardrail.py	Adds contract tests for `Verdict`/`Refusal`/`Envelope` serialization and `policy()` behavior.
tests/test_guardrail_audit.py	Adds tests for `guardrail_audit` migration + best-effort write behavior.
tests/test_guard_context.py	Updates guard-context parity tests to compare `Verdict
tests/test_execute_sql_envelope.py	Adds focused tests for executor stderr refusal parsing into a refused `Envelope`.
tests/test_contracts.py	Removes retired `ErrorResult` coverage and adds `GuardrailAuditRecord` roundtrip tests.
tests/test_column_scope_gate.py	Updates column-scope and `SELECT *` gate tests to the new `Verdict
tests/test_column_scope_adversarial.py	Updates adversarial scope tests to match the `Verdict
tests/test_ace051_fail_closed.py	Updates fail-closed tests to assert `{"refusal": ...}` instead of `{"error": ...}`.
tests/test_ace044_bounded_fetch.py	Updates bounded-fetch assertions to `Envelope.data.truncated` and `Envelope.applied`.
tests/e2e/test_safety_envelope.py	Adds cross-transport E2E test ensuring both stdio and HTTP return the same `Envelope` shape.
plugins/agami/lib/sql_guard.py	Refactors read-only guard to return a safety `Verdict` wrapper over `_read_only_reason()`.
plugins/agami/lib/guardrail.py	Adds vendored guardrail contract module for plugin distribution.
plugins/agami/lib/execute_sql.py	Emits `{"refusal": ...}` to stderr and maps safety gate verdicts to refusal lines.
packages/agami-core/src/tools.py	Assembles/returns `Envelope`, parses executor refusal lines, and records guardrail audit at a shared chokepoint.
packages/agami-core/src/sql_guard.py	Refactors read-only guard to return `Verdict
packages/agami-core/src/semantic_model/runtime.py	Updates safety gates to return `Verdict
packages/agami-core/src/ports.py	Retires `GovernanceVerdict` and re-types governance seam to `list[Verdict]`.
packages/agami-core/src/oss_adapters.py	Updates OSS governance adapter to return an empty `list[Verdict]`.
packages/agami-core/src/model_store.py	Adds DB sink method `record_guardrail_audit(...)`.
packages/agami-core/src/guardrail.py	Adds core guardrail contract module (vendored stdlib-only).
packages/agami-core/src/execute_sql.py	Emits `{"refusal": ...}` to stderr and maps safety gate verdicts to refusal lines (core executor).
packages/agami-core/src/contracts.py	Retires ad-hoc execute_sql error contract and adds `GuardrailAuditRecord`.
packages/agami-core/pyproject.toml	Adds `guardrail` to flat `py-modules`.
migrations/core/012_guardrail_audit.sql	Adds `guardrail_audit` table + index.
dev.py	Updates vendored-lib drift guard to include `guardrail.py`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    OSS default = warn-only, and returns no findings ("basic governance warning"); enforcement is
+    a paid tier that supplies its own adapter."""


 class WarnOnlyGovernancePolicy:
-    """Default ``GovernancePolicy`` — never blocks (``allowed=True``); any warnings are advisory
-    ("basic governance warning"). Enforcement is a paid tier."""
+    """Default ``GovernancePolicy`` — emits no governance findings (an empty ``Verdict`` list),
+    so nothing is warned, rewritten, or blocked. Enforcement is a paid tier that supplies its own
+    adapter returning governance-class ``Verdict``s."""


+    """The ``refusal`` block of a refused ``Envelope``: why the query was rejected, with a
+    remediation. Keep raw SQL and raw DB error text out of ``reason`` — it crosses the
+    boundary between the model and the customer's database, and error-text sanitization is
+    hardened separately."""


+    """The ``refusal`` block of a refused ``Envelope``: why the query was rejected, with a
+    remediation. Keep raw SQL and raw DB error text out of ``reason`` — it crosses the
+    boundary between the model and the customer's database, and error-text sanitization is
+    hardened separately."""


sandeep-agami · 2026-07-12T01:45:51Z

Addressed the Copilot review (all four were docstring-accuracy comments, no behavior change) in the latest commit:

ports.py GovernancePolicy — removed the "warn-only, and returns no findings" contradiction. The OSS default is a no-op (empty list); the warn/recommend/enforce tier posture is applied by policy(), not the adapter.
oss_adapters.py WarnOnlyGovernancePolicy — clarified that the name is the OSS posture (governance only ever warns, never enforces) while the default adapter, having no rules wired, emits no findings. I kept the name rather than renaming to NoopGovernancePolicy: it's pre-existing and its empty-warnings behavior is unchanged by this PR (the old GovernanceVerdict(allowed=True, warnings=[]) was also empty), and it's referenced across mcp_http + tests. Happy to rename in a follow-up if you'd prefer.
3 & 4. Refusal docstring (core + vendored copy) — now distinguishes a guardrail refusal (clean reason, never raw SQL/DB text) from an operational failure (carries the driver's error text until sanitization lands separately), matching _refusal_from_stderr.

Full gate green after the change.

The one typed result shape for the SQL guardrails — Verdict, policy(verdict, tier), and the response Envelope (ok | refused) with a nested ExecuteSqlResult payload + audit_id. The safety gates (check_read_only + the runtime scope/PII checks) return a Verdict; execute_sql returns one Envelope on every surface; a guardrail_audit row is recorded at the single chokepoint. The governance plug-point is kept (re-typed to Verdict); the data-protection / governance policy branches are stubbed for F10/F11. Reconciled onto the execute_guarded executor seam (AH-012) + the in-process default (ACE-028) that merged to main while this was in review: - execute_guarded returns the typed Envelope and raises GuardRefused carrying a typed Refusal (replacing the placeholder {"error": ...} dict); _model_safety RETURNS its Refusal instead of writing it to stderr. - So the in-process path (tools._run_in_process) gets the SAME typed refusal the subprocess path does — closing the follow-up main deferred ("_model_safety to return its envelope"): a model-safety refusal keeps its structured detail on BOTH surfaces instead of degrading to a generic string in-process. - One success shape too: _finalize_execution builds Envelope.ok(data=...) for both the subprocess and in-process paths; subprocess main() emits {"refusal": ...} to stderr from the typed refusal (the byte-identical wire the tool relays). Full gate green (1525 tests); the seam / fail-closed / envelope / audit tests updated to the unified typed contract, plus a conftest reset for the process-global injected executor so a seam test can't leak into a subprocess-path test. Spec: ACE-035 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.

        truncated = True
    return columns, data_rows, truncated
+def _refusal_from_stderr(stderr: str | None, returncode: int) -> Refusal:
+    """Build a Refusal from the executor's stderr. A guardrail refusal is a `{"refusal": {...}}`


@@ -1015,20 +1074,24 @@
    envelope is identical either way (a guard refusal's envelope is not yet identical across paths —
    see `_finalize_execution` and the tracked structured-refusal-parity follow-up).


Review-response to the Agami panel + Copilot on the reconciled guardrail contract. No behavior change to the guard decision; observability, docstring accuracy, naming, and test precision. - Audit observability (panel should-fix): the guardrail-audit sinks were best-effort AND silent, so a persistently dead sink (a DB role without INSERT, an unwritable log dir) lost the security record undetectably. Add a one-time (per-process, per-key) value-free stderr warning in _finish (record-build failure) and _record_guardrail_audit (DB-sink error + jsonl-fallback failure); the query stays best-effort. Conftest resets the dedup set per test. - Refusal docstring (Copilot): state the value-free-reason contract for BOTH guardrail and operational refusals (raw driver text goes to the audit trail only), instead of the stale "operational failures carry raw text" note. - Governance naming (Copilot): rename WarnOnlyGovernancePolicy -> NoopGovernancePolicy (the OSS default emits no findings; the name now matches), fix the contradictory ports/module docstrings, and move the annotation-only Verdict import under TYPE_CHECKING. - tool_execute_sql docstring (Copilot): both paths now build the same typed refused Envelope; drop the stale "not yet identical across paths" note. Add the E305 blank lines before _refusal_from_stderr. - _resolve_guard_model (panel): a store.close() error no longer discards a model that loaded fine (false refusal); the loader import moved inside the disk-path try so an import failure fails closed. - REFUSAL_KINDS (panel): document that unscopable_sql/resource_limit/recon are forward-declared for the dependent slices, so the emit-site test's emitted-subset-of-documented direction is intended. Tests: real PII gate -> refused Envelope (kind sensitive_columns, end-to-end, executor never runs); HTTP refusal writes a guardrail_audit row (audit-on-both-surfaces, read back from the table); column-scope adversarial tests assert Verdict.rule == "column_scope" (gate identity); audit sink failure warns exactly once (both jsonl + DB branches), value-free. Spec: ACE-035 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings July 12, 2026 01:17

sandeep-agami requested a review from ashwin-agami as a code owner July 12, 2026 01:17

Copilot started reviewing on behalf of sandeep-agami July 12, 2026 01:18 View session

Copilot AI reviewed Jul 12, 2026

View reviewed changes

sandeep-agami mentioned this pull request Jul 12, 2026

ACE-037: fail-closed on unscopable SQL #112

Open

5 tasks

ashwin-agami approved these changes Jul 12, 2026

View reviewed changes

This was referenced Jul 12, 2026

Per-statement timeout for the SQL executor (resource_limit) #115

Open

Recon deny-list + DB-error sanitization for the SQL guard #120

Open

sandeep-agami force-pushed the ACE-035-shared-guardrail-contract branch from bd445a0 to 3950f08 Compare July 12, 2026 17:39

sandeep-agami requested a review from Copilot July 13, 2026 01:24

Copilot started reviewing on behalf of sandeep-agami July 13, 2026 01:25 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

sandeep-agami mentioned this pull request Jul 14, 2026

ACE-041: PII masking (mask-else-refuse) #128

Draft

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ACE-035: shared guardrail contract — Verdict / policy / Envelope#111

ACE-035: shared guardrail contract — Verdict / policy / Envelope#111
sandeep-agami wants to merge 2 commits into
mainfrom
ACE-035-shared-guardrail-contract

sandeep-agami commented Jul 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

sandeep-agami commented Jul 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		OSS default = warn-only, and returns no findings ("basic governance warning"); enforcement is
		a paid tier that supplies its own adapter."""

		@@ -1015,20 +1074,24 @@
		envelope is identical either way (a guard refusal's envelope is not yet identical across paths —
		see `_finalize_execution` and the tracked structured-refusal-parity follow-up).

Uh oh!

Conversation

sandeep-agami commented Jul 12, 2026

Summary

Changes

Design decisions

Test plan

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

sandeep-agami commented Jul 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants