Skip to content

Expose transaction_aml_check.amlResponsible on /gs/debug - #4393

Merged
TaprootFreak merged 1 commit into
developfrom
feat/debug-aml-responsible
Jul 27, 2026
Merged

Expose transaction_aml_check.amlResponsible on /gs/debug#4393
TaprootFreak merged 1 commit into
developfrom
feat/debug-aml-responsible

Conversation

@TaprootFreak

Copy link
Copy Markdown
Collaborator

Why

transaction_aml_check is the audit trail behind every amlCheck transition. On the structured
/gs/debug endpoint it could answer when a transition happened and which code path caused it
(AmlSourceType), but never who approved itamlResponsible was unreachable.

That column was blocked by the startup invariant forbidding any overlap between
DebugAllowedColumns and GsRestrictedColumns (the per-role masking list /gs/db applies).
The invariant is sound as a default, but it made "who released this transaction" unanswerable
from the audit trail, which is exactly what AML forensics needs.

What

  • transaction_aml_check.amlResponsible is now allowlisted for /gs/debug.
  • New DebugRestrictedOverlapExceptions: an explicit registry of (table, column) pairs that may
    overlap the masking list. The invariant now permits only registered pairs; every other
    overlap still aborts module load, so a future addition still cannot slip in silently.
  • A counter-check rejects stale exception entries (column dropped from either list), so the
    registry cannot rot into a silent gap.
  • The invariant moved into the pure, exported assertDebugAllowlistInvariants(...) and is covered
    by synthetic fixtures.

Scope of the exposure

  • /gs/db masking is unchanged. GsRestrictedColumns is untouched, so callers below
    SuperAdmin still see [RESTRICTED] there.
  • comment on the same table stays blocked — free-form internal note text, no exception entry,
    invariant still rejects it.
  • Audience is the /gs/debug role gate: Debug, plus Admin / SuperAdmin via the role
    hierarchy in role.guard.ts.
  • Every call remains attributable: executeDebugQuery audit-logs caller address and query.
  • No new inference path. GROUP BY / ORDER BY resolve only allowlisted columns or select
    aliases (emitDebugGroupIdent emits ordinals to sidestep Postgres name resolution), so the
    newly visible value cannot be used as an oracle onto columns that remain blocked.

Why the guard was extracted

Asserting the real constants only proves today's data is consistent. It cannot show that the
exception matches per column rather than per table — with the current data there is no
second overlap left to expose such a bug. If the check were ever loosened to table level, a later
accidental re-add of comment would pass silently. The fixtures drive the guard directly,
including the case where excepting one column must not amnesty a second overlap in the same table.

Tests

gs.service.spec.ts: 353 passing (8 new fixture tests for the guard, 4 for the exposure itself).
The existing "rejects sensitive column" matrix keeps transaction_aml_check.comment;
amlResponsible was removed from it deliberately and replaced by dedicated coverage.

Review notes

This deliberately widens what a Debug-role caller can read. The security argument is the role
gate plus the audit log, not obscurity — worth a conscious call from the compliance side rather
than a routine approval.

The structured debug endpoint could show that an amlCheck transition happened and
which code path caused it, but never who approved it: amlResponsible was blocked
by the startup invariant that forbids any overlap with the /gs/db masking list.
That left "who released this transaction" unanswerable from the audit trail.

Allowlist the column and introduce DebugRestrictedOverlapExceptions, an explicit
registry of (table, column) pairs that may overlap. Every unregistered overlap
still aborts module load, and a counter-check rejects stale entries so the list
cannot rot into a silent gap. /gs/db masking is untouched, and `comment` on the
same table stays blocked.

Extract the invariant into the pure, exported assertDebugAllowlistInvariants so
the guard itself is testable: asserting the real constants only proves today's
data is consistent, not that the exception matches per column rather than per
table. Covered by synthetic fixtures, including the case where excepting one
column must not amnesty a second overlap in the same table.
@TaprootFreak

Copy link
Copy Markdown
Collaborator Author

Pre-review completed in 2 rounds before this PR was opened, with two independent reviewers per
round (conformance against CONTRIBUTING.md, and logic/correctness/security), each on its own
working copy.

Round 1 — 1 finding (MEDIUM): the new exception logic was not covered in isolation. The
assertions only compared the real constants against each other, which proves today's data is
consistent but not that the guard matches per (table, column) rather than per table. With no
second overlap left in the data, a later loosening to table level — and with it an accidental
re-add of comment — would have passed unnoticed.

Fix: the invariant was extracted into the pure, exported assertDebugAllowlistInvariants(...)
and is now driven by synthetic fixtures: unregistered overlap throws, both stale-entry directions
throw, a non-debuggable table throws, and excepting one column does not amnesty a second overlap
in the same table. Prototype-chain keys use computed properties, since a plain __proto__: in an
object literal sets the prototype instead of an own property.

Round 2 — 0 findings from both reviewers on the fixed diff.

One open argument from round 2 was closed separately: whether the newly visible value could serve
as an oracle through GROUP BY / aggregates. emitDebugGroupIdent / emitDebugOrderIdent resolve
only allowlisted columns or select aliases, and GROUP BY emits ordinal positions to sidestep
Postgres' input-column-over-alias resolution — so columns that remain blocked stay unreachable.

Local verification: 353/353 in gs.service.spec.ts, tsc --noEmit, ESLint and Prettier clean.
CI: 11/11 green.

@TaprootFreak
TaprootFreak marked this pull request as ready for review July 26, 2026 17:02
@TaprootFreak
TaprootFreak merged commit 3595ff1 into develop Jul 27, 2026
11 checks passed
@TaprootFreak
TaprootFreak deleted the feat/debug-aml-responsible branch July 27, 2026 06:52
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.

1 participant