Skip to content

ACE-097: a reachable but unmigrated audit store executes unrecorded, and relays driver text to the caller #192

Description

@sandeep-agami

Summary

ACE-097's guarantee is that a server which cannot record does not execute. It holds when the audit
store is unreachable. It does not hold when the store is reachable but unmigrated.

_audit_store_reachable() establishes that the store can be opened, and treats that as
establishing it can be written. Those are not the same for a fresh or partially-migrated database.
So a brand-new app database passes the pre-execution gate, the statement runs against the warehouse,
and the recording INSERT then fails on the missing table. The call returns error / kind: other
carrying the raw driver message in remediation, which is caller-facing.

Two problems, and the second is independent of the first:

  1. The statement executes and nothing records it. Measured, not inferred (see below).
  2. Raw driver text reaches the caller. ACE-039 splits these deliberately: raw text is the
    operator's, on _RAW_LOG; the classified sentence is the caller's. Here the driver's message,
    including a fragment of the internal INSERT, lands in the caller-facing remediation field.
    Any later recording-write failure takes the same route, so this is a channel rather than one
    message.

This is reachable in a normal deployment, not only in a test rig: mcp_http applies pending
migrations in its startup lifespan (packages/agami-core/src/mcp_http.py, fail-closed), and
mcp_harness has no migration call at all. A stdio deployment pointed at a fresh app database
therefore executes unrecorded on every call until an HTTP boot heals the schema.

Not a regression. Reproduced identically on main @ 86278a0.

Repro

  1. A warehouse database with a table, a read-only role, and a semantic model introspected from it.
  2. An app database a role can connect to that has never been migrated:
psql -d postgres -c "CREATE ROLE app_owner LOGIN PASSWORD 'apw';" \
                 -c "CREATE DATABASE app_unmigrated OWNER app_owner;"
  1. Drive the stdio surface against it:
AGAMI_DB_URL="postgresql://app_owner:apw@localhost:5432/app_unmigrated" \
  python -m mcp_harness      # then tools/call execute_sql with any SELECT

Expected vs actual

Expected (what the unreachable-store case already does): refused / audit_unavailable, the
statement never reaches the warehouse, and the driver text goes to _RAW_LOG only.

Actual: the statement runs, no row is recorded, and the response is

{"error": {"kind": "other",
           "remediation": "relation \"query_executions\" does not exist\nLINE 1: INSERT INTO query_executions (id, ts, org_id, datasource, qu...\n"}}

Evidence

Same call, same model, PostgreSQL 16.12. xact_commit is read on the warehouse database before
and after, so execution is measured from the engine's own counter rather than from our logs.

Arm Response Warehouse xact_commit delta
healthy migrated store (control) status=ok, rows returned 2
reachable, unmigrated store error / other + raw driver text 2
unreachable store (role cannot connect) refused / audit_unavailable 0

The third row is the behaviour that already works, which is why this reads as a gap in the gate's
definition of reachable rather than as a missing gate.

A note on measuring this, because two attempts at the control were wrong before this one. With no
semantic model present, the call refuses before it ever executes and the warehouse delta reads 0 on
both arms, which looks exactly like the guarantee holding. It is the harness measuring nothing. The
control has to produce a non-zero delta or it is not a control.

Suggested direction

Two changes, not mutually exclusive:

  • Have the gate establish writability rather than connectivity, so an unmigrated store refuses like
    an unreachable one.
  • Classify a failed recording write as audit_unavailable rather than other, keeping the driver
    text on _RAW_LOG. This half is worth doing on its own: it closes the leak channel for every
    write failure, not just this one.

Worth deciding alongside: whether mcp_harness should migrate on open the way mcp_http does, or
whether refusing is the right answer for a surface that does not own the schema.

Whatever the fix, it needs a regression test that fails on today's code, with a control that produces
a non-zero warehouse delta.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions