Skip to content

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

Open
sandeep-agami wants to merge 2 commits into
ACE-038-resource-limits-timeoutfrom
ACE-039-recon-error-hardening
Open

Recon deny-list + DB-error sanitization for the SQL guard#120
sandeep-agami wants to merge 2 commits into
ACE-038-resource-limits-timeoutfrom
ACE-039-recon-error-hardening

Conversation

@sandeep-agami

Copy link
Copy Markdown
Collaborator

Summary

Closes two information-leak channels that remain open even under read-only + object-scope:

  1. Recon / metadata deny-list — a new sql_guard.check_no_recon gate refuses queries that fingerprint the server (version(), current_user, inet_server_addr()), enumerate or dump the system catalog (information_schema, pg_catalog, any pg_* relation — including pg_stats sampled VALUES and pg_authid password hashes), read catalog DDL (pg_get_viewdef/pg_get_functiondef), or probe object existence (has_table_privilege, to_regclass). The read-only role can't revoke catalog access on most engines, so this is the intended app-side backstop. A distinct recon refusal, wired at the single executor chokepoint (execute_sql.py::main) + a fail-fast tool-layer pre-check, so both transports inherit it.

  2. DB-error sanitization — a raw driver error returned to the caller leaks schema, column names, or data values across the assistant boundary. The tool layer now classifies an operational failure into a value-free message (rich kind mirroring db_error_classifier.md), and the raw detail goes only to a new guardrail_audit.error_detail audit column, keyed by audit_id.

Changes

  • sql_guard.py: check_no_recon + data-driven recon sets, regex over the same neutralized SQL as check_read_only (no second parser). pg_ is a reserved relation/function prefix (mirrors the Redshift stl_/… prefixes); qualified columns (t.pg_class) pass via a (?<!\.) lookbehind. Fails closed when the SQL can't be neutralized.
  • execute_sql.py / tools.py: wire the recon gate; _classify_db_error + _refusal_from_stderr returns (Refusal, error_detail); the raw stderr is threaded to the audit row only.
  • guardrail.py: add network/column_not_found/table_not_found to REFUSAL_KINDS (+ the drift test now enumerates every emitted kind).
  • model_store.py / store.py / migration 013: the error_detail column + a graceful INSERT degrade (survive a pre-013 schema instead of dropping the audit row) + Store.rollback.

Verification

  • Recon corpus enumerates every deny-list member (self-updating) + the pg_ leak closures + neutralization bypasses (comment/case/quoted-identifier/set-op/subquery); a false-positive corpus pins the legit look-alikes (bare version/user/schema columns, t.pg_class, tokens in literals/comments, window fns/CTEs) with the accepted residuals pinned explicitly.
  • Sanitization corpus asserts the raw token is absent from the entire envelope JSON across every kind + dialect + the high-leak host/user cases, and the audit both-ends (raw in error_detail, absent from the response).
  • Full test gate green (1655 passed); driven end-to-end through the executor CLI (pg_stats/pg_authid/pg_get_viewdef/has_table_privilege/inet_server_addr()recon; a qualified o.pg_class column returns data).

Review

Ran the Agami review panel (correctness · silent-failure · test-quality · High-lane security). Error-sanitization sign-off satisfied; the panel found real primary-engine recon false-negatives (the pg_ catalog relations + introspection functions) and a pre-013 audit-row-drop — all fixed here, with the corpus expanded to prove them closed.

Checklist

  • Spec: ACE-039
  • Both surfaces (stdio + HTTP) via the single execute_sql/tool_execute_sql chokepoint
  • Vendored plugins/agami/lib in sync (dev.py sync-lib; drift-checked)
  • Full gate green; no raw DB text on any Envelope path (security-verified)
  • Human PR approval

Stacked on #115; retarget to main once #111 / #112 / #115 merge.

Spec: ACE-039

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new “recon / metadata” safety gate to the SQL guardrail pipeline and prevents operational DB error text from leaking across the assistant boundary by sanitizing refusals while storing raw details server-side in the guardrail audit trail.

Changes:

  • Introduces sql_guard.check_no_recon and wires it at the executor chokepoint and tool-layer precheck to refuse catalog/introspection/fingerprinting queries as recon.
  • Sanitizes DB operational failures into value-free refusal messages via _classify_db_error, while capturing raw stderr/messages into a new guardrail_audit.error_detail column (with jsonl fallback).
  • Extends guardrail refusal vocabulary and expands/updates test corpora to pin both security and false-positive behavior.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_sql_guard.py Adds recon deny-list corpus (must-refuse + must-pass) and executor e2e recon refusal assertion.
tests/test_guardrail.py Updates refusal-kind drift test to include new recon and refined operational kinds.
tests/test_guardrail_audit.py Verifies raw operational error text is stored only in audit (DB/jsonl) and never in the response envelope; adds pre-013 degrade test.
tests/test_execute_sql_envelope.py Replaces exit-code-only behavior with stderr classification + sanitization assertions across multiple dialect wordings.
tests/test_contracts.py Updates GuardrailAuditRecord roundtrip sample to include error_detail.
tests/test_ah012_executor_seam.py Ensures injected in-process executor path also sanitizes operational errors (no raw detail in refusal reason).
plugins/agami/lib/sql_guard.py Vendors check_no_recon and recon regex sets into plugin library copy.
plugins/agami/lib/guardrail.py Updates documented REFUSAL_KINDS to include new operational kinds.
plugins/agami/lib/execute_sql.py Wires recon gate into executor guarded pipeline (plugin library copy).
packages/agami-core/src/tools.py Adds recon precheck, implements _classify_db_error, threads error_detail to audit only, and sanitizes in-process path parity.
packages/agami-core/src/store.py Adds Store.rollback() to support retrying inserts after Postgres transaction aborts.
packages/agami-core/src/sql_guard.py Implements core check_no_recon deny-list and regex matching over neutralized SQL.
packages/agami-core/src/model_store.py Adds error_detail audit insert with fallback for pre-013 schemas (with rollback).
packages/agami-core/src/guardrail.py Extends REFUSAL_KINDS with network, column_not_found, table_not_found.
packages/agami-core/src/execute_sql.py Wires recon gate into core executor guarded pipeline.
packages/agami-core/src/contracts.py Extends GuardrailAuditRecord contract with error_detail.
migrations/core/013_guardrail_audit_error_detail.sql Adds guardrail_audit.error_detail column for server-side raw driver text retention.

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

Comment thread packages/agami-core/src/model_store.py Outdated
Comment on lines +377 to +381
except Exception:
# Pre-013 schema (the error_detail column isn't there yet, e.g. new code against an
# un-migrated DB): DON'T drop the audit row — write it without the raw detail. Roll back
# first so a Postgres aborted-transaction doesn't fail the retry.
self._store.rollback()
@sandeep-agami
sandeep-agami force-pushed the ACE-038-resource-limits-timeout branch from bd84345 to 17fe585 Compare July 13, 2026 02:09
sandeep-agami and others added 2 commits July 12, 2026 19:11
Two hardening gates at the shared execution chokepoint (execute_guarded), so both the
subprocess wire and the in-process handler inherit them identically.

1. Recon / metadata deny-list. sql_guard.check_no_recon refuses server-fingerprinting and
   system-catalog introspection (version(), current_user, information_schema, pg_* relations)
   as a distinct `recon` refusal — wired right after the read-only guard, a hard gate NOT
   bypassable via --no-safety.

2. DB-error sanitization. A driver failure can echo schema / column / value names (or a DSN)
   in its message. Both surfaces now classify an operational failure into a VALUE-FREE refusal
   (_classify_db_error → fixed _ERROR_KINDS reason + remediation) and route the RAW driver text
   to the server-side audit trail ONLY, via a new guardrail_audit.error_detail column
   (migration 013). The subprocess path does this in _refusal_from_stderr; the in-process path
   now does the same in _run_in_process (returns (Refusal, error_detail)) — closing the leak
   the AH-012 seam had surfaced by riding exc.msg on the reason.

Rebased onto the reconciled ACE-038 seam: the recon gate raises the typed GuardRefused; the
two AH-012 seam tests that asserted the old raw-message-in-reason behavior now assert the
sanitized, value-free contract (parity with the subprocess wire).

Spec: ACE-039

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…3 case only

Copilot review: DbActivitySink.record_guardrail_audit retried the INSERT on ANY exception, then
re-ran it without error_detail. A first-INSERT failure for a NON-schema reason (a deadlock, a
constraint violation, a dropped connection) would spuriously trigger the fallback — masking the
real DB error AND silently dropping error_detail. Narrow the retry to the missing-error_detail-column
signature only (sqlite "no column", Postgres "does not exist" / UndefinedColumn); any other failure
re-raises to the best-effort caller (which now warns once, per the ACE-035 review fix).

Test pins that a non-schema error propagates on the first attempt — one execute call, no rollback,
original error re-raised — while the existing pre-013 degrade test still passes.

Spec: ACE-039

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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.

2 participants