From 06bf47e6c20e9e692eac5a5452d5805d742d01e8 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 27 Jul 2026 19:05:45 +0200 Subject: [PATCH] Add filter-only columns to /gs/debug and allow user_data.mail lookups (#4400) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add filter-only columns to /gs/debug and allow user_data.mail lookups The structured debug endpoint governed select, where, order by and group by through a single per-table allowlist, so a column was either fully exposed or not exposed at all. Support needs to resolve a customer's userData.id from a mail address they already have, while the endpoint must never be able to return a mail address. Introduce DebugTableSpec.filterOnlyColumns: columns usable only as a WHERE leaf, restricted to = and IN via DebugFilterOnlyAllowedOps. Range and pattern operators stay rejected, because they would let a caller binary-search or pattern-match an unknown value and reconstruct it without ever selecting it. Split assertDebugColumnAllowed into a select-side and a filter-side check; order by and group by keep validating against columns only, which is what keeps a filter-only column unorderable and ungroupable. Extend the startup invariants so filter-only columns stay disjoint from columns and jsonbColumns and free of duplicates, and widen the staleness counter-check to accept either allowlist capacity - without that, a restricted column could never be configured as filter-only, since the two checks would contradict each other. Allowlist user_data.mail as the first filter-only column and add a --user-by-mail mode to scripts/db-debug.sh returning all matching ids, since one address can belong to several user_data rows. * Harden filter-only columns: equality only, no NOT, case-insensitive An independent review of the initial implementation found that the operator gate could be bypassed and that the guarantee was weaker than documented. The WHERE tree supports a `not` node, so `NOT (mail = x)` emitted the semantic equivalent of the prohibited `mail != x`, sidestepping the gate entirely. A filter-only column is now refused anywhere below a `not`, at any nesting depth and regardless of double negation - no attempt is made to reason about negations cancelling out. Drop `IN` from the allowed operators. It was never unsafe in itself, but a list of up to 100 candidates per request multiplied the throughput of a guessing attack; equality alone forces one request per candidate, each separately audit-logged. Emit filter-only equality as `LOWER(col) = LOWER($n)`. The application itself resolves mail addresses that way, because historical user_data rows carry mixed-case values - a strict `=` would have silently failed to find exactly the records this feature exists for. Equality stays equality: the caller must still know the address, only letter case is forgiven. The client printed the request payload, putting the address into terminal scrollback and captured output. Payload echoes now redact WHERE values for every mode, mirroring the server's audit-log redaction; query structure stays visible and the transmitted body is unchanged. * Pin NOT propagation through boolean nodes, document LOWER precondition Second review round. The negated flag is propagated into `and` / `or` children and the behaviour was correct, but nothing tested it - a regression that stopped propagating through a boolean node would have stayed green. The added cases cover NOT over AND, over OR and over a deeper nesting, plus the positive control `AND [ NOT (id = 1), mail = x ]`, where the filter-only leaf sits outside the negation and must be accepted. That last one is what pins the semantics rather than a blanket refusal. Case-insensitive emission presupposes a text column: a non-text filter-only column would fail at query time with `function lower(integer) does not exist`. A real type check would need schema introspection, so the precondition is documented at the declaration and at the emission site instead. Correct three inaccurate claims in the docs: the endpoint emits SQL manually with bound parameters rather than through a QueryBuilder; the caller must know the address exactly except for letter case; and queries are audit-logged with their WHERE values redacted, not verbatim. * Limit a query to one filter-only predicate; audit before the table check Dropping `IN` was meant to force one guessed address per request, but a single query may carry up to 50 WHERE leaves, so `OR(mail='a', mail='b', ...)` batched candidates just as effectively and binary-splitting the set found matches in few requests. The guarantee was therefore not enforced. A query now accepts at most one filter-only predicate anywhere in the WHERE tree, nested boolean nodes included; the comments state the actual reasoning instead of crediting the `IN` removal alone. Emit the audit line before the table allowlist check, so a request naming a non-allowlisted table is attributable too - previously it threw first and was never logged, while the docs claimed every request is audited. Values stay redacted. Correct two comments that still described a TypeORM QueryBuilder, and reword the allowlist policy comments: PII is excluded from selectable and result columns, with explicitly reviewed filter-only exceptions such as user_data.mail. Add a rejecting test for the offset upper bound, which the existing test could not catch because it bypasses the validation pipe. * Make the alias test actually probe alias resolution; fix audit claim The test covering ORDER BY / GROUP BY of an alias named like a filter-only column never reached those clauses: it failed during SELECT validation, so it would have stayed green with the alias checks deleted. It now selects `id AS mail`, so the alias really registers, and asserts that GROUP BY resolves to the alias ordinal and ORDER BY to the quoted alias, with neither binding to the physical user_data.mail column. Correct the documented audit guarantee: requests rejected by the global ValidationPipe never reach the service and are therefore not audit-logged. Coverage is every request that passes DTO validation, including those later rejected for an unknown table or a disallowed column - the document previously contradicted itself on this point. Sweep the remaining comments claiming the debug endpoint uses a TypeORM QueryBuilder; the /gs/db and support paths genuinely do use one and are left untouched. * Log database failures by error code, never by message The endpoint redacts WHERE values in its audit log, but the query-execution catch block logged the raw database error, and PostgreSQL echoes offending parameter values in some of them. Filtering `mail = 'x@example.com'` together with `id = 'x@example.com'` makes PostgreSQL reject the second parameter with `invalid input syntax for type integer: "x@example.com"`, so the address reached the log in clear text - defeating the redaction for exactly the value it protects. Failures are now logged as value-free diagnostics: SQLSTATE plus severity and routine where available, with an explicit placeholder when no code is present rather than falling back to the message. The response to the caller is unchanged. The existing test demanded the raw message and would have stayed green while values leaked, so it now asserts the value-free form. A regression test feeds an error whose message embeds an address and asserts no log line contains it while the code is still reported. * Bind an overlap exception to the access it approved Widening the staleness check to accept either allowlist capacity left a hole: an exception approving a restricted column for equality-only lookup stayed valid after someone moved that column from filterOnlyColumns into columns, silently upgrading it to full disclosure. The exception recorded the pair but not the access that had been reviewed. Approvals are now capacity-specific. DebugRestrictedOverlapExceptions keeps its original meaning - selectable access - and its staleness check is restored accordingly, so the existing transaction_aml_check.amlResponsible entry is unaffected. A separate DebugFilterOnlyRestrictedExceptions covers filter-only access and is empty today, since no filter-only column is restricted. Registering the same pair in both is rejected, because an approval has to be unambiguous. Moving a column between the two lists therefore breaks module load until the approval is moved as well, which is the visible step this is meant to force. The log-redaction test also inspected only the first logger argument, so passing the error object as a second one would have leaked the address through its stack while the test stayed green; it now pins the argument count too. * Correct the rationale for case-insensitive equality and the index claim The justification given for `LOWER(col) = LOWER($n)` was wrong. It cited historical mixed-case rows, but migration NormalizeUserDataMailLowercase - already in the base - lowercased every stored address, and the write paths lowercase on input. The behaviour is right for a different reason: it matches the identity the application itself uses, since getUsersByMail resolves via LOWER(mail), and it tolerates a caller typing a different case than stored. The performance note claimed the comparison cannot use an index. The opposite holds: the same migration creates a functional index on LOWER(mail), which is exactly what this emission uses. That index is deliberately non-unique while case-collision duplicates await a merge campaign - which is also why one address can legitimately resolve to several user_data rows. The /gs/db masking test asserted only the contents of GsRestrictedColumns, so it would have passed even if masking had stopped being applied. It now runs getDbData for both roles and checks that ADMIN sees [RESTRICTED] while SUPER_ADMIN sees the value. * Fix the type-check failure and use absolute imports The DbQueryDto fixture added for the /gs/db masking test omitted the required identifier property, so `tsc --noEmit` failed and the CI check went red. It stayed invisible locally because `nest build` excludes test files and Jest transpiles without type checking - both were green throughout. The fixture now carries every required property with a real value rather than being cast into shape, since a fixture that only type-checks through an assertion defeats its own purpose. The two imports this branch introduced also used relative paths, which CONTRIBUTING disallows; they are absolute now, matching the surrounding files. * Keep the mail address and the request body out of process argv The script promised that a customer address never appears outside the request payload, but argv is world-readable through ps and /proc, and the address reached it three ways: as the positional argument of --user-by-mail, through `jq --arg`, and inside `curl -d "$PAYLOAD"`. The address is now read as one line from stdin - prompted on stderr when stdin is a TTY so an operator can answer interactively, read silently otherwise so pipes keep working. Empty input or EOF fails loudly instead of querying with an empty filter. The value reaches jq through stdin. Request bodies go to curl via `-d @-` for every mode, not only the new one: the other modes bind user ids and asset names the same way, so fixing only this one would have left the same exposure in place and the guarantee only half true. What is sent is unchanged. Help text and documentation now describe the stdin form and state the guarantee precisely. * Accept stdin without a trailing newline; reject stray arguments; state the guarantee precisely `printf %s address | … --user-by-mail` failed even though a complete value had been read: Bash `read` returns non-zero at EOF without a trailing newline. The read is now treated as successful whenever it produced a value, with the empty/whitespace rejection unchanged. Trailing arguments were silently dropped, so `--user-by-mail 50 --typo` used 50 and ignored the typo. Unconsumed arguments are now rejected and the limit is validated as a positive integer, matching the script's fail-loud discipline. Three statements promised more than the code delivers and are now exact. Examples no longer pipe through `echo`, which could place the address in an external echo's argv - the very exposure the stdin input removes; they show interactive entry or reading from a protected file. Interactive entry is documented as visible on the terminal, which is accepted since the operator is typing an address they already know. And enabling TypeORM query logging on the server would print bound parameters, defeating the audit redaction; it is off in production, and that condition is now written down instead of implied. * Scope the argv guarantee to what holds; fix the help example and limit range Request bodies now reach curl through stdin, but `--query ''` still takes the payload as an argument, so an inline DTO stays visible in the process list and in shell history. The form is kept - it is convenient for ordinary queries - and the documentation now says plainly that sensitive values, in particular a filter-only column, must go through `--query @file` or `--query -` instead. `--user-by-mail` is unaffected, it reads from stdin. The `--help` output rendered its heredoc terminator indented, so the printed direct-API example could not be pasted and run. It is emitted correctly now. `--user-by-mail [N]` accepted any positive integer while the server caps limit at 10000, so an oversized value authenticated and was then rejected remotely. The client validates 1..10000 and names the range in the help text and docs. * Match the sibling's secret handling; fix two argument-validation bugs The equivalent command in the team tooling reads the address hidden and keeps values out of its error text, while this script still echoed the address on a TTY and reflected submitted limits, trailing arguments and a positionally mistyped address back to stderr. Two scripts serving the same purpose gave different guarantees. Interactive entry is now hidden with a newline after the prompt, piped input is unchanged, and every error in this mode reports only the kind of problem plus how to supply the address. Two validation bugs came out of the same review. A long digit string passed the regex but overflowed the arithmetic comparison, which emitted `integer expected`, evaluated false and let the script continue to authentication despite the promised client-side rejection; the digit length is now bounded before any arithmetic. And trailing arguments were detected by testing whether the third argument was non-empty, so `--user-by-mail 50 "" ignored` slipped through while an explicitly empty limit was mistaken for an omitted one; both are now decided by argument count. * Pin the value-validation branches of a filter-only equality filter The test rejecting a non-scalar WHERE value only submitted an object, so the array branch of that check could have been removed unnoticed. Filter-only equality also had no coverage for an omitted value, null, an empty array, a single-element array or an empty string - security-relevant branches that were correct but unpinned. The added cases are table-driven and, for every input that must be rejected, also assert that no query was issued: a rejection that still reached the database would be a real defect that asserting only the thrown error would miss. The empty string is pinned as accepted on purpose - it is a legitimate exact filter that simply matches nothing. A synthetic fixture now also pins an empty filterOnlyColumns list as a valid configuration. --- scripts/DEBUG-ENDPOINT.md | 104 +- scripts/db-debug.sh | 209 +++- skills/db-debug/SKILL.md | 28 +- skills/db-debug/reference.md | 59 +- .../generic/gs/__tests__/gs.service.spec.ts | 1064 ++++++++++++++++- .../generic/gs/dto/debug-query.dto.ts | 7 +- src/subdomains/generic/gs/dto/gs.dto.ts | 181 ++- src/subdomains/generic/gs/gs.controller.ts | 5 +- src/subdomains/generic/gs/gs.service.ts | 127 +- 9 files changed, 1655 insertions(+), 129 deletions(-) diff --git a/scripts/DEBUG-ENDPOINT.md b/scripts/DEBUG-ENDPOINT.md index 135172a2bc..f5224d39ab 100644 --- a/scripts/DEBUG-ENDPOINT.md +++ b/scripts/DEBUG-ENDPOINT.md @@ -85,6 +85,14 @@ with the wallet at `DEBUG_ADDRESS`. # Referral chain / tree for a userDataId ./scripts/db-debug.sh --referral-chain 370625 ./scripts/db-debug.sh --referral-tree 370625 + +# Resolve user_data id(s) from a mail you already know (filter-only; mail is never returned). +# Address on stdin — not in process argv; script does not print it; errors never echo +# submitted values; payload echo redacts WHERE values. Prefer interactive entry or a +# protected file over piping via `echo` (which would put the address in echo's argv). +./scripts/db-debug.sh --user-by-mail # interactive: prompts "Mail address (input hidden): " +./scripts/db-debug.sh --user-by-mail < address.txt +./scripts/db-debug.sh --user-by-mail 50 < address.txt ``` For ad-hoc queries the endpoint expects a JSON DTO (no raw SQL). See the @@ -94,19 +102,100 @@ per-table column allowlist lives in `src/subdomains/generic/gs/dto/gs.dto.ts` (`DebugAllowedColumns`); a column absent from a table's entry is unreachable from this endpoint. +### Filter-only columns + +Most allowlisted columns may appear in `select` / `where` / `orderBy` / `groupBy` and accept +the full ordinary operator set. A **filter-only** column is narrower: it may appear only as a +WHERE leaf, only with `=` (not `IN` — batching multiplies guessing throughput; one address +per request, each separately audit-logged), never under a `NOT` node at any depth (including +double negation: `NOT (mail = x)` is semantically `mail != x`), and never in select, order by, +or group by. Equality is case-insensitive (`LOWER(col) = LOWER($n)`): it matches the +application's own case-insensitive address identity (`getUsersByMail` resolves via +`LOWER(mail)`), so the debug lookup answers the same question the application asks, and +it tolerates the caller typing an address in a different case than stored. The intent is +lookup by a value the caller already knows, without the endpoint ever disclosing that +value. + +The first instance is `user_data.mail` (`filterOnlyColumns` on the `user_data` entry in +`DebugAllowedColumns`). Support needs to resolve a customer's `userData.id` from an address +they already have; the endpoint must not become a way to read addresses out. Selecting +`mail` (or ordering / grouping by it) is rejected with 400. One mail can map to several +`user_data` rows — the result is an array of matching rows (do not clamp `limit` to 1). + +Convenience mode and equivalent raw DTO: + +```bash +# Interactive (TTY prompts on stderr with input hidden) or from a protected file — +# avoid `echo … |` so the address never lands in an external process's argv. +./scripts/db-debug.sh --user-by-mail +./scripts/db-debug.sh --user-by-mail < address.txt + +# Same query as an ad-hoc --query payload (mail bound as a JSON value; never returned). +# Prefer --query @file or --query - (stdin / heredoc) so the address is not in the shell's +# command line. An inline `--query ''` places the full DTO — including the mail value +# — in the script's own argv and in shell history; do not use that form for sensitive values +# such as filter-only columns (e.g. user_data.mail). --user-by-mail is unaffected: it always +# reads the address from stdin. +./scripts/db-debug.sh --query - <<'EOF' +{ + "table": "user_data", + "select": [ + {"kind": "column", "column": "id"}, + {"kind": "column", "column": "created"}, + {"kind": "column", "column": "kycLevel"}, + {"kind": "column", "column": "status"} + ], + "where": {"kind": "leaf", "column": "mail", "op": "=", "value": "user@example.com"}, + "orderBy": [{"column": "id", "direction": "ASC"}], + "limit": 100 +} +EOF +``` + +**Client argv and redaction guarantee (`scripts/db-debug.sh`):** Request bodies are sent to +`curl` via stdin (`-d @-`), so they never appear in curl's process arguments. That is not a +blanket guarantee for every form of input: an inline `--query ''` still places the +complete DTO (including any value inside it) in the script's own argv and in shell history. +For ordinary non-sensitive queries that is fine; for any sensitive value — in particular a +filter-only column such as `user_data.mail` — use `--query @file` or `--query -` (stdin) +instead. `--user-by-mail` reads the address from stdin (not a positional argument). The +address is passed into `jq` via stdin (not `--arg`) and every request body is sent with +`curl … -d @-`. Under that mode the address is not placed in any process's argv. The script +does not print the address; error messages are value-free (no submitted limit, trailing arg, +or address is echoed); and the payload echo redacts WHERE leaf values (`` / +``, matching `serializeDebugQueryForAudit`). At a TTY the prompt states that input +is hidden and read uses echo-off, so the address does not enter terminal scrollback; pipes +use plain read. Audit-log and error-path redaction hold under normal production config +(`SQL_LOGGING` unset, so TypeORM query logging is off — see the comment in +`src/shared/services/typeorm-logger.ts`). Enabling SQL query logging (`SQL_LOGGING`) makes +TypeORM print bound parameters — including the address — for successful queries, which +defeats that redaction. Optional `[N]` on `--user-by-mail` is an integer in `1..10000` +(server DTO cap); default remains 100; extra arguments are rejected by count. + ## Security Notes 1. **Never commit** `.env` to git (it's in `.gitignore`) 2. The DEBUG role should only be granted to authorized personnel -3. All queries are logged with user identifier for audit (`Debug-query by : …`). - WHERE leaf values are redacted in the audit log; structure (table / columns / ops) is preserved. +3. Every debug request that passes DTO validation and reaches the service is audit-logged with + the caller identifier (`Debug-query by : …`) before emit/execute — including ones later + rejected for an unknown table or a disallowed column. Requests rejected by NestJS' + ValidationPipe (e.g. `limit: 0`, invalid `kind`) never reach the service and produce no + audit line. The log is the redacted DTO structure (table / select / where ops / columns); + WHERE leaf values are replaced with `` / `` and never logged verbatim. + Failed executions get a separate `… failed:` info line with value-free diagnostics only + (SQLSTATE `code`, and when present `severity` / `routine`). The raw database error + message is never logged — Postgres may echo bound parameter values in it, which would + defeat WHERE-value redaction. Missing `code` is logged as `code=`. + 4. The endpoint accepts a structured JSON DTO only — no raw SQL is parsed, walked, or interpolated. 5. Identifiers (table, column, alias, aggregate, op, ORDER BY direction, jsonb path segment) - are validated against an allowlist before being interpolated into SQL; values are bound as - `$1..$N` parameters via TypeORM. + are validated against an allowlist before being interpolated into a hand-built SQL string; + values are bound as `$1..$N` parameters and executed via `dataSource.query` (not QueryBuilder). 6. Tables and columns reachable from this endpoint are enumerated in `DebugAllowedColumns` (`src/subdomains/generic/gs/dto/gs.dto.ts`). Anything not listed there is unreachable; - PII / secrets / free-form text are deliberately excluded. + PII / secrets / free-form text are deliberately excluded. Filter-only columns + (`filterOnlyColumns`, e.g. `user_data.mail`) may be used only as WHERE leaves with `=` + (not `IN`, not under `NOT`; equality is case-insensitive) and are never returned in results. ### Kill switch / revocation @@ -126,8 +215,11 @@ absent from a table's entry is unreachable from this endpoint. ### "Query execution failed" for database - Verify the table is listed in `DebugAllowedColumns` -- Verify every referenced column appears in that table's `columns` array +- Verify every referenced column appears in that table's `columns` array (or, for WHERE only, + in `filterOnlyColumns` — filter-only columns cannot be selected / ordered / grouped) - jsonb path access (the `jsonb` select kind) is allowed only on columns listed in `jsonbColumns` (currently only `log.message`) +- Filter-only columns accept only `=` in WHERE (not `IN`, not under any `NOT`); equality is + case-insensitive; other operators are rejected - If the JSON body is malformed at the DTO level (wrong `kind`, missing required field, value out of range) NestJS' ValidationPipe rejects with a 400 before the service runs diff --git a/scripts/db-debug.sh b/scripts/db-debug.sh index e5e75f5f06..65f976a610 100755 --- a/scripts/db-debug.sh +++ b/scripts/db-debug.sh @@ -10,6 +10,7 @@ # ./scripts/db-debug.sh --asset-history Yapeal/EUR 10 # Show asset balance history # ./scripts/db-debug.sh --referral-chain # Show referral chain # ./scripts/db-debug.sh --referral-tree # Show referral tree +# ./scripts/db-debug.sh --user-by-mail [N] # Resolve user_data id(s); mail on stdin # ./scripts/db-debug.sh --get [cols] [limit] # Ad-hoc: fetch cols from any allowlisted table # ./scripts/db-debug.sh --query '|@file|-' # Ad-hoc: POST an arbitrary structured DTO # @@ -26,12 +27,29 @@ # Safety: # - DEBUG_API_URL defaults to PRODUCTION. The endpoint is read-only by construction: # it accepts a JSON query description and emits parameter-bound SELECT statements -# through TypeORM. Writes / DDL are not expressible. +# via dataSource.query. Writes / DDL are not expressible. +# - Request bodies are sent to curl via stdin (`-d @-`), never as a curl argv value, so the +# body does not appear in curl's process arguments (`ps` / `/proc/*/cmdline`) on a shared +# host. That does not cover every way a payload can still sit in argv: an inline +# `--query ''` places the complete DTO — including any value inside it — in this +# script's own argv and in shell history. Inline is fine for ordinary, non-sensitive +# queries; for any sensitive value (in particular a filter-only column such as +# user_data.mail) use `--query @file` or `--query -` (stdin) instead. +# - --user-by-mail is unaffected: it reads the address from stdin (not argv), so it does +# not appear in any process's argv. The script does not print the address; error messages +# are value-free (no submitted limit, trailing arg, or address is echoed); the payload +# echo redacts WHERE values the same way the server audit log does. At a TTY the prompt +# states that input is hidden and read uses echo-off, so the address does not enter +# terminal scrollback; pipes use plain read. Audit-log and error-path redaction hold +# under normal production config (SQL_LOGGING unset). Enabling SQL query logging +# (SQL_LOGGING) makes TypeORM print bound parameters — including the address — for +# successful queries, which defeats that redaction (see the comment in +# src/shared/services/typeorm-logger.ts). # # Structured /gs/debug endpoint: # The endpoint no longer accepts raw SQL. The request body is a JSON description of -# the query that the service translates into SQL via TypeORM QueryBuilder with bound -# parameters. Shape: +# the query that the service translates into SQL manually with bound parameters and +# executes via dataSource.query. Shape: # # { # "table": "log", @@ -69,9 +87,27 @@ # Columns default to id,created,updated; limit defaults to 100. # ./scripts/db-debug.sh --get user_data # ./scripts/db-debug.sh --get buy_crypto id,created,amountInEur 50 +# --user-by-mail [N] +# Resolve user_data id(s) from a mail address you already know. The address is read from +# stdin (one line; TTY → prompt on stderr with input hidden; non-TTY → silent read). +# Empty/EOF fails loudly. Filters on the filter-only column user_data.mail (= only, +# case-insensitive, not under NOT); never selects mail. One address can match several +# rows. Limit defaults to 100 (not 1) and must be an integer in 1..10000 (server DTO +# cap); trailing args are rejected by argument count. Returns id, created, kycLevel, +# status. The address is not placed in any process's argv; the script does not print it; +# error messages never echo submitted values; the payload echo redacts WHERE values. +# Audit/error redaction holds when SQL query logging is off (production: SQL_LOGGING +# unset); enabling SQL_LOGGING would print bound parameters including the address. +# ./scripts/db-debug.sh --user-by-mail # interactive: input hidden at prompt +# ./scripts/db-debug.sh --user-by-mail < address.txt +# ./scripts/db-debug.sh --user-by-mail 50 < address.txt # --query '' | --query @path/to/query.json | --query - # Posts an arbitrary DebugQueryDto. Accepts inline JSON, @file, or - to read the DTO from stdin. # The DTO is validated as well-formed JSON (jq) before the request; malformed JSON fails loudly. +# Inline `--query ''` is convenient for ordinary non-sensitive queries, but the full +# DTO (including any values) is visible in this script's argv and shell history. For any +# sensitive value — e.g. a filter-only column such as user_data.mail — use --query @file +# or --query - (stdin) instead. --user-by-mail already reads the address from stdin. # ./scripts/db-debug.sh --query '{"table":"asset","select":[{"kind":"column","column":"name"}],"limit":5}' # ./scripts/db-debug.sh --query @/tmp/query.json # echo '{"table":"asset","select":[{"kind":"column","column":"id"}],"limit":1}' | ./scripts/db-debug.sh --query - @@ -117,10 +153,24 @@ if [ "${1:-}" = "-h" ] || [ "${1:-}" = "--help" ]; then echo " Show complete referral chain for user" echo " -T, --referral-tree " echo " Show complete referral tree (all branches)" + echo " -M, --user-by-mail [N] Resolve user_data id(s) from a known mail on stdin" + echo " (default limit: 100, integer 1..10000; no extra args)." + echo " mail is filter-only: usable only in WHERE with =" + echo " (case-insensitive; not under NOT; no IN), never returned." + echo " One mail can match several rows. Address is not in any" + echo " process argv; script does not print it; errors never echo" + echo " submitted values; payload echo redacts WHERE values." + echo " At a TTY the prompt hides typed input (no scrollback);" + echo " pipes use plain read. Audit/error redaction holds when" + echo " SQL query logging is off (prod: SQL_LOGGING unset);" + echo " enabling SQL_LOGGING would print bound parameters" + echo " incl. the address." echo " -g, --get
[cols] [N] Ad-hoc: fetch cols (default id,created,updated) from any" echo " allowlisted table (default limit: 100)" echo " -q, --query Ad-hoc: POST an arbitrary structured DTO (inline JSON," - echo " @file, or - to read the DTO from stdin)" + echo " @file, or - to read the DTO from stdin). Inline puts the" + echo " full DTO in this script's argv and shell history; for" + echo " sensitive values (e.g. user_data.mail) use @file or -." echo "" echo "Examples:" echo " ./scripts/db-debug.sh --anomalies 50" @@ -130,6 +180,9 @@ if [ "${1:-}" = "-h" ] || [ "${1:-}" = "--help" ]; then echo " ./scripts/db-debug.sh --asset-history MaerkiBaumann/CHF 10" echo " ./scripts/db-debug.sh --referral-chain 370625" echo " ./scripts/db-debug.sh --referral-tree 370625" + echo " ./scripts/db-debug.sh --user-by-mail # interactive: input hidden at prompt" + echo " ./scripts/db-debug.sh --user-by-mail < address.txt" + echo " ./scripts/db-debug.sh --user-by-mail 50 < address.txt" echo " ./scripts/db-debug.sh --get user_data" echo " ./scripts/db-debug.sh --get buy_crypto id,created,amountInEur 50" echo " ./scripts/db-debug.sh --query '{\"table\":\"asset\",\"select\":[{\"kind\":\"column\",\"column\":\"name\"}],\"limit\":5}'" @@ -140,7 +193,9 @@ if [ "${1:-}" = "-h" ] || [ "${1:-}" = "--help" ]; then echo " curl -X POST \$API_URL/gs/debug \\" echo " -H 'Authorization: Bearer \$TOKEN' \\" echo " -H 'Content-Type: application/json' \\" - echo " -d '{\"table\":\"asset\",\"select\":[{\"kind\":\"column\",\"column\":\"id\"},{\"kind\":\"column\",\"column\":\"name\"}],\"orderBy\":[{\"column\":\"id\",\"direction\":\"DESC\"}],\"limit\":5}'" + echo " -d @- <<'EOF'" + echo "{\"table\":\"asset\",\"select\":[{\"kind\":\"column\",\"column\":\"id\"},{\"kind\":\"column\",\"column\":\"name\"}],\"orderBy\":[{\"column\":\"id\",\"direction\":\"DESC\"}],\"limit\":5}" + echo "EOF" exit 0 fi @@ -304,6 +359,84 @@ case "${1:-}" in REFERRAL_TREE_MODE="1" TARGET_USER_ID="$2" ;; + -M|--user-by-mail) + # Mail is read from stdin (not argv) so it does not appear in process argv via ps/proc. + # Optional limit stays positional (matches other modes). Empty/EOF fails loudly — no + # default filter. Address is bound into jq via stdin (not --arg); the shared payload + # echo redacts WHERE values (see serializeDebugQueryForAudit); DESCRIPTION and this + # branch's own messages must not reintroduce the address either. Error messages are + # value-free: never echo a submitted limit, trailing arg, or address. + # + # Validate by argument count ($#), not emptiness of $2/$3: an explicitly empty second + # arg is invalid, and "--user-by-mail 50 '' ignored" must still be rejected. + if [ "$#" -gt 2 ]; then + echo "Error: unexpected argument — address comes from stdin (one line), not as an argument" + echo "Usage: ./scripts/db-debug.sh --user-by-mail [N]" + exit 1 + fi + if [ "$#" -eq 2 ]; then + # Server DebugQueryDto caps limit at 10000; reject out-of-range client-side so a large + # N fails before auth instead of after. Default remains 100. + # Bound digit length before arithmetic: a long digit string passes a loose regex but + # overflows bash integer comparison ("integer expected") and would otherwise proceed. + if ! [[ "$2" =~ ^[1-9][0-9]{0,4}$ ]] || [ "$2" -gt 10000 ]; then + echo "Error: --user-by-mail limit must be an integer in 1..10000" + echo "Usage: ./scripts/db-debug.sh --user-by-mail [N]" + exit 1 + fi + USER_BY_MAIL_LIMIT="$2" + else + USER_BY_MAIL_LIMIT="100" + fi + if [ -t 0 ]; then + printf 'Mail address (input hidden): ' >&2 + # Echo off: address must not land in terminal scrollback. Newline after so following + # output is not glued to the prompt. + IFS= read -r -s USER_BY_MAIL || true + printf '\n' >&2 + else + # One line; EOF without content → empty. Trailing newline stripped by read -r. + IFS= read -r USER_BY_MAIL || true + fi + # Trim surrounding whitespace only (trailing newline already removed by read). + # Do not lowercase — the server matches case-insensitively. + USER_BY_MAIL="${USER_BY_MAIL#"${USER_BY_MAIL%%[![:space:]]*}"}" + USER_BY_MAIL="${USER_BY_MAIL%"${USER_BY_MAIL##*[![:space:]]}"}" + if [ -z "$USER_BY_MAIL" ]; then + echo "Error: --user-by-mail requires a non-empty mail address on stdin (one line)" + echo "Usage: ./scripts/db-debug.sh --user-by-mail [N]" + echo "" + echo "Pass the address on stdin (one line). Interactive TTY prompts on stderr with" + echo "input hidden; pipes/scripts pass the line without a prompt. Empty input / EOF fails." + echo "" + echo "Examples:" + echo " ./scripts/db-debug.sh --user-by-mail # interactive prompt (input hidden)" + echo " ./scripts/db-debug.sh --user-by-mail < address.txt" + echo " ./scripts/db-debug.sh --user-by-mail 50 < address.txt" + echo "" + echo "Resolves user_data id(s) from a mail you already know. mail is filter-only:" + echo "usable only as a WHERE leaf with = (case-insensitive; not under NOT; no IN)," + echo "never selectable. One mail can match several user_data rows; default limit is 100" + echo "(allowed range for N: 1..10000)." + exit 1 + fi + # Pass the address via stdin into jq (same pattern as --query JSON validation), not + # --arg — so the address is not in jq's argv either. + PAYLOAD=$(printf '%s' "$USER_BY_MAIL" | jq -R --argjson limit "$USER_BY_MAIL_LIMIT" '{ + table: "user_data", + select: [ + {kind: "column", column: "id"}, + {kind: "column", column: "created"}, + {kind: "column", column: "kycLevel"}, + {kind: "column", column: "status"} + ], + where: {kind: "leaf", column: "mail", op: "=", value: .}, + orderBy: [{column: "id", direction: "ASC"}], + limit: $limit + }') + DESCRIPTION="user_data by mail (filter-only, limit $USER_BY_MAIL_LIMIT)" + OUTPUT_MODE="objects" + ;; -g|--get) if [ -z "${2:-}" ]; then echo "Error: --get requires a table name" @@ -392,9 +525,10 @@ API_URL="${DEBUG_API_URL:-https://api.dfx.swiss/v1}" # --- Authenticate --- echo "=== Authenticating to $API_URL ===" -TOKEN_RESPONSE=$(curl -s -X POST "$API_URL/auth" \ +# Body via stdin (`-d @-`) so credentials are not in curl's argv (ps /proc visibility). +TOKEN_RESPONSE=$(printf '%s' "{\"address\":\"$DEBUG_ADDRESS\",\"signature\":\"$DEBUG_SIGNATURE\"}" | curl -s -X POST "$API_URL/auth" \ -H "Content-Type: application/json" \ - -d "{\"address\":\"$DEBUG_ADDRESS\",\"signature\":\"$DEBUG_SIGNATURE\"}") + -d @-) TOKEN=$(echo "$TOKEN_RESPONSE" | jq -r '.accessToken' 2>/dev/null) @@ -413,6 +547,14 @@ echo "" # of objects). This filter rebuilds that shape so we can reuse them. KEYS_ROWS_TO_OBJECTS='[ .keys as $k | .rows[] | [$k, .] | transpose | map({(.[0]): .[1]}) | add ]' +# --- Helper: POST a JSON body to /gs/debug without putting the body in curl argv --- +post_debug_query() { + printf '%s' "$1" | curl -s -X POST "$API_URL/gs/debug" \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -d @- +} + # --- Helper: build a payload for a single-leaf where query --- # Used by the referral walkers. build_leaf_payload() { @@ -459,10 +601,7 @@ if [ -n "$REFERRAL_CHAIN_MODE" ]; then # Query recommendation for current user SELECT_JSON='[{"kind":"column","column":"recommenderId"},{"kind":"column","column":"method"},{"kind":"column","column":"created"}]' REQ_PAYLOAD=$(build_leaf_payload "recommendation" "$SELECT_JSON" "recommendedId" "$CURRENT_ID" "null" "1") - RESULT=$(curl -s -X POST "$API_URL/gs/debug" \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$REQ_PAYLOAD") + RESULT=$(post_debug_query "$REQ_PAYLOAD") ROWS_LEN=$(echo "$RESULT" | jq -r '.rows | length // 0' 2>/dev/null) if [ "$ROWS_LEN" = "0" ] || [ -z "$ROWS_LEN" ]; then @@ -532,10 +671,7 @@ if [ -n "$REFERRAL_TREE_MODE" ]; then ROOT_ID="$CURRENT_ID" SELECT_JSON='[{"kind":"column","column":"recommenderId"}]' REQ_PAYLOAD=$(build_leaf_payload "recommendation" "$SELECT_JSON" "recommendedId" "$CURRENT_ID" "null" "1") - RESULT=$(curl -s -X POST "$API_URL/gs/debug" \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$REQ_PAYLOAD") + RESULT=$(post_debug_query "$REQ_PAYLOAD") ROWS_LEN=$(echo "$RESULT" | jq -r '.rows | length // 0' 2>/dev/null) if [ "$ROWS_LEN" = "0" ] || [ -z "$ROWS_LEN" ]; then CURRENT_ID="" @@ -562,10 +698,7 @@ if [ -n "$REFERRAL_TREE_MODE" ]; then # Get user status local status_select='[{"kind":"column","column":"status"},{"kind":"column","column":"kycStatus"}]' local status_payload=$(build_leaf_payload "user_data" "$status_select" "id" "$user_id" "null" "1") - local status_result=$(curl -s -X POST "$API_URL/gs/debug" \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$status_payload") + local status_result=$(post_debug_query "$status_payload") local status=$(echo "$status_result" | jq -r '.rows[0][0] // "?"') local kyc_status=$(echo "$status_result" | jq -r '.rows[0][1] // "?"') @@ -590,10 +723,7 @@ if [ -n "$REFERRAL_TREE_MODE" ]; then local children_select='[{"kind":"column","column":"recommendedId"}]' local children_order='[{"column":"created","direction":"ASC"}]' local children_payload=$(build_leaf_payload "recommendation" "$children_select" "recommenderId" "$user_id" "$children_order" "1000") - local children_result=$(curl -s -X POST "$API_URL/gs/debug" \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$children_payload") + local children_result=$(post_debug_query "$children_payload") local children=$(echo "$children_result" | jq -r '.rows[]?[0] // empty' 2>/dev/null) if [ -n "$children" ]; then @@ -635,10 +765,7 @@ if [ -n "$REFERRAL_TREE_MODE" ]; then for check_id in $TO_CHECK; do count_select='[{"kind":"column","column":"recommendedId"}]' count_payload=$(build_leaf_payload "recommendation" "$count_select" "recommenderId" "$check_id" "null" "1000") - count_children_result=$(curl -s -X POST "$API_URL/gs/debug" \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$count_payload") + count_children_result=$(post_debug_query "$count_payload") count_children=$(echo "$count_children_result" | jq -r '.rows[]?[0] // empty' 2>/dev/null) for child in $count_children; do if [[ ! " $KNOWN_IDS " =~ " $child " ]]; then @@ -685,10 +812,7 @@ if [ -n "$ASSET_HISTORY_MODE" ]; then }, limit: 1 }') - ASSET_RESULT=$(curl -s -X POST "$API_URL/gs/debug" \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$ASSET_PAYLOAD") + ASSET_RESULT=$(post_debug_query "$ASSET_PAYLOAD") ASSET_ID=$(echo "$ASSET_RESULT" | jq -r '.rows[0][0] // empty' 2>/dev/null) @@ -709,13 +833,26 @@ fi echo "=== Executing Debug Query ===" echo "Query: $DESCRIPTION" echo "Payload:" -echo "$PAYLOAD" | jq -c . +# Redact WHERE leaf values the same way the server redacts its audit log +# (serializeDebugQueryForAudit): scalar → "", array → "". +# Structure (table / columns / ops) stays visible; the request body is unredacted. +echo "$PAYLOAD" | jq -c ' + walk( + if type == "object" and has("value") then + .value = + if (.value | type) == "array" then + "" + else + "" + end + else + . + end + ) +' echo "" -RESULT=$(curl -s -X POST "$API_URL/gs/debug" \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$PAYLOAD") +RESULT=$(post_debug_query "$PAYLOAD") echo "=== Result ===" diff --git a/skills/db-debug/SKILL.md b/skills/db-debug/SKILL.md index 234054bde8..06191c898c 100644 --- a/skills/db-debug/SKILL.md +++ b/skills/db-debug/SKILL.md @@ -1,6 +1,6 @@ --- name: db-debug -description: Read-only debugging of the production database via the scripts/db-debug.sh CLI (the structured /gs/debug endpoint). Use to inspect financial anomalies, total-balance history (FinancialDataLog), liquidity balances, an asset's balance history, referral chains or trees, compare two balance-log snapshots, inspect one asset's balance structure, or run an ad-hoc read-only query against any allowlisted table with a structured JSON DTO. No raw SQL — read-only by construction, never writes. +description: Read-only debugging of the production database via the scripts/db-debug.sh CLI (the structured /gs/debug endpoint). Use to inspect financial anomalies, total-balance history (FinancialDataLog), liquidity balances, an asset's balance history, referral chains or trees, resolve user_data id(s) from a known mail (filter-only), compare two balance-log snapshots, inspect one asset's balance structure, or run an ad-hoc read-only query against any allowlisted table with a structured JSON DTO. No raw SQL — read-only by construction, never writes. --- # Database debug (read-only) @@ -9,7 +9,8 @@ Read-only forensics against the production database through the `scripts/db-debu script authenticates itself (DEBUG address + signature from the local `.env`) and posts a **structured JSON query** to the `/gs/debug` endpoint. There is no raw SQL: the request body is a `DebugQueryDto` describing table, select items, an optional where-tree, group/order/limit, which the -service compiles to a parameter-bound SELECT via TypeORM. Writes and DDL are not expressible, and +service emits as a hand-built SELECT with bound `$1..$N` parameters (executed via +`dataSource.query`, not QueryBuilder). Writes and DDL are not expressible, and every identifier must appear in the per-table allowlist (`DebugAllowedColumns` in `src/subdomains/generic/gs/dto/gs.dto.ts`) — the source of truth, which drifts per migration. @@ -39,15 +40,21 @@ request implies a write, refuse and explain why. | `--asset-history [N]` | balance history for one asset (default 10) | | `--referral-chain ` | referral chain upward | | `--referral-tree ` | full referral tree with status | +| `--user-by-mail [N]` | resolve `user_data` id(s) from a known mail on stdin (filter-only; mail never returned; not in process argv; script does not print it; errors never echo submitted values; payload echo redacts WHERE values; TTY prompt hides typed input; audit/error redaction holds when SQL query logging is off / prod `SQL_LOGGING` unset; default limit 100, integer 1..10000, trailing args rejected by count; one mail can match several rows) | | `--get
[col1,col2,...] [limit]` | ad-hoc: fetch columns (default `id,created,updated`) from any allowlisted table (default limit 100) | -| `--query ` | ad-hoc: POST an arbitrary structured DTO (inline JSON, `@file`, or `-` to read the DTO from stdin) | +| `--query ` | ad-hoc: POST an arbitrary structured DTO (inline JSON, `@file`, or `-` to read the DTO from stdin). Inline puts the full DTO in the script's argv and shell history; for sensitive values (e.g. `user_data.mail`) use `@file` or `-` | | `--help` | full usage | ## Ad-hoc queries Use `--get` for the common case (columns from one table) and `--query` for anything that needs a where-tree, aggregate, jsonb path, group by, or order by. `--query` validates that its payload is -well-formed JSON (via `jq`) and fails loudly before contacting the endpoint. +well-formed JSON (via `jq`) and fails loudly before contacting the endpoint. Request bodies go to +`curl` via `-d @-` (not in curl's argv), but an **inline** `--query ''` still places the +complete DTO — including any value inside it — in the script's own argv and in shell history. +Inline is fine for ordinary non-sensitive queries; for any sensitive value (in particular a +filter-only column such as `user_data.mail`) use `--query @file` or `--query -` (stdin). +`--user-by-mail` always reads the address from stdin and is unaffected by that caveat. ``` # Latest 100 rows of a table, default columns id,created,updated @@ -56,10 +63,19 @@ scripts/db-debug.sh --get user_data # Specific columns and a limit scripts/db-debug.sh --get buy_crypto id,created,amountInEur 50 -# Arbitrary DTO inline +# Resolve user_data id(s) from a known mail on stdin (filter-only; mail is never returned). +# Address on stdin — not in process argv; script does not print it; errors never echo +# submitted values; payload echo redacts WHERE values. Prefer interactive entry or a +# protected file over piping via `echo` (which would put the address in echo's argv). +# Optional N is integer 1..10000 (default 100); trailing args rejected by count. +scripts/db-debug.sh --user-by-mail # interactive: prompts "Mail address (input hidden): " +scripts/db-debug.sh --user-by-mail < address.txt +scripts/db-debug.sh --user-by-mail 50 < address.txt + +# Arbitrary DTO inline (non-sensitive values only — full DTO is in argv / shell history) scripts/db-debug.sh --query '{"table":"asset","select":[{"kind":"column","column":"id"},{"kind":"column","column":"name"}],"where":{"kind":"leaf","column":"blockchain","op":"=","value":"Ethereum"},"orderBy":[{"column":"id","direction":"DESC"}],"limit":20}' -# Arbitrary DTO from a file, or from stdin +# Arbitrary DTO from a file, or from stdin (prefer these for sensitive values such as mail) scripts/db-debug.sh --query @/tmp/query.json cat query.json | scripts/db-debug.sh --query - ``` diff --git a/skills/db-debug/reference.md b/skills/db-debug/reference.md index f08fd93df0..4142505ea9 100644 --- a/skills/db-debug/reference.md +++ b/skills/db-debug/reference.md @@ -7,15 +7,30 @@ the TypeORM entities in this repository. ## Endpoint & safety - The CLI posts a `DebugQueryDto` (structured JSON) to `POST /gs/debug` with a Bearer token obtained from `POST /auth` (DEBUG address + signature from the local `.env`, role `DEBUG`). -- No raw SQL crosses the wire. The service (`src/subdomains/generic/gs`) compiles the DTO to SQL via - TypeORM QueryBuilder: every identifier (table, column, alias, aggregate, op, order-by direction, - jsonb path segment) is validated against an allowlist, and all leaf values are bound as parameters - (`$1..$N`) — never interpolated. Writes / DDL are not expressible; read-only is structural. +- No raw SQL crosses the wire. The service (`src/subdomains/generic/gs`) emits SQL manually: every + identifier (table, column, alias, aggregate, op, order-by direction, jsonb path segment) is + validated against an allowlist, and all leaf values are bound as parameters (`$1..$N`) — never + interpolated — then executed with `dataSource.query` (not QueryBuilder). Writes / DDL are not + expressible; read-only is structural. - Reachable tables and columns are enumerated in `DebugAllowedColumns` (`src/subdomains/generic/gs/dto/gs.dto.ts`) — the **source of truth**. It drifts per migration: every migration that adds, renames, or removes a column on a debuggable table updates it. A table or column absent from that map is unreachable (PII / secrets / free-form text are deliberately excluded). The full DTO schema is `src/subdomains/generic/gs/dto/debug-query.dto.ts`. +- **Filter-only columns** (`filterOnlyColumns` on a table's `DebugTableSpec`): usable only as a + WHERE leaf, never in select / order by / group by, and only with `=` (not `IN` — batching + multiplies guessing throughput; one address per request, each separately audit-logged). Range, + inequality, and pattern ops would turn the endpoint into an oracle. A filter-only column may + not appear under a `NOT` node at any depth, including double negation (`NOT (mail = x)` is + semantically `mail != x`). Equality is case-insensitive (`LOWER(col) = LOWER($n)`): it + matches the application's own case-insensitive address identity (`getUsersByMail` + resolves via `LOWER(mail)`), so the debug lookup answers the same question the + application asks, and it tolerates the caller typing an address in a different case + than stored. No `jsonbPath`. Intended for looking a record up by a value the + caller already knows, without the endpoint ever disclosing that value. First instance: + `user_data.mail` — resolve `userData.id`(s) from a known address; selecting `mail` is refused. + One mail can map to several `user_data` rows; use a multi-row `limit` (e.g. 100), never 1. + Ordinary allowlisted columns keep the full operator set; only filter-only columns are restricted. - The default target is production (`DEBUG_API_URL` in the local `.env`). - `limit` is required (1..10000); the service additionally clamps to its own max. Page larger scans with explicit `limit` + `offset`. @@ -29,15 +44,20 @@ the TypeORM entities in this repository. segments are dot-separated and each is regex-validated. - `{"kind":"aggregate","aggregate":"count|sum|min|max|avg","column":"id","as":"n"}`. - optional `as` on any item sets the output alias (also referenceable in `orderBy` / `groupBy`). + - Filter-only columns must not appear in `select` (plain, aggregate, or jsonb). - `where` (optional): a tree of nodes, each with a `kind`: - `{"kind":"leaf","column":"x","op":"=","value":...}` — ops: `= != < <= > >= IN "NOT IN" LIKE ILIKE "IS NULL" "IS NOT NULL"`. `IN` / `NOT IN` take an array value; `IS NULL` / `IS NOT NULL` - take no value; the rest take a scalar. + take no value; the rest take a scalar. On filter-only columns only `=` is allowed (not `IN`, + not under any `NOT` node at any depth); equality is case-insensitive + (`LOWER(col) = LOWER($n)`). Ordinary allowlisted columns keep the full operator set above. - `{"kind":"and","children":[...]}` / `{"kind":"or","children":[...]}` — up to 5 children each. - `{"kind":"not","child":{...}}`. - Caps: tree depth ≤ 5, ≤ 200 nodes, ≤ 50 leaf predicates, ≤ 100 values per IN list. -- `groupBy` (optional): array of columns or select-aliases (order preserved). -- `orderBy` (optional): array of `{"column":"x","direction":"ASC|DESC"}` (column or select-alias). +- `groupBy` (optional): array of columns or select-aliases (order preserved); filter-only columns + are not allowed. +- `orderBy` (optional): array of `{"column":"x","direction":"ASC|DESC"}` (column or select-alias); + filter-only columns are not allowed. - `limit` (required); `offset` (optional, ≥ 0). - Column names are camelCase and case-sensitive; table names are snake_case. - Response shape: `{"keys":[...],"rows":[[...], ...]}` — `keys` mirror the `as`-or-column order of @@ -78,6 +98,29 @@ Entity: `src/subdomains/core/liquidity-management/entities/liquidity-balance.ent ## Other useful tables - `recommendation`: `recommenderId`, `recommendedId`, `method`, `created` (referrals). -- `user_data`: `id`, `status`, `kycStatus`, … (used by referral-tree status lookups). +- `user_data`: `id`, `status`, `kycStatus`, `kycLevel`, … (used by referral-tree status lookups). + `mail` is **filter-only** (not in `columns`): WHERE `=` only (case-insensitive; not under + `NOT`; no `IN`), never selected / ordered / grouped. CLI: + `scripts/db-debug.sh --user-by-mail [N]` (default limit 100, integer 1..10000; trailing args + rejected by count; interactive TTY prompts on stderr with input hidden) or + `scripts/db-debug.sh --user-by-mail [N] < address.txt`. Prefer interactive entry or a + protected file — do not pipe via `echo` (that would put the address in echo's argv). The + address is read from stdin, passed into `jq` via stdin (not `--arg`), and request bodies go + to `curl` via `-d @-` — not in any process argv under that mode. An **inline** + `--query ''` is different: the complete DTO (including any mail value inside it) sits + in the script's own argv and in shell history. For hand-built mail predicates, use + `--query @file` or `--query -` (stdin / heredoc), not inline JSON. `--user-by-mail` is + unaffected (address always from stdin). The script does not print the address; error + messages are value-free (no submitted limit, trailing arg, or address is echoed); the + payload echo redacts WHERE values. At a TTY the prompt states that input is hidden and + read uses echo-off, so the address does not enter terminal scrollback; pipes use plain + read. Audit-log and error-path redaction hold under normal production config + (`SQL_LOGGING` unset, so TypeORM query logging is off — see + `src/shared/services/typeorm-logger.ts`). Enabling SQL query logging (`SQL_LOGGING`) makes + TypeORM print bound parameters — including the address — for successful queries, which + defeats that redaction. Equivalent DTO (post via `@file` or `-`, never as an inline argv + string when it contains a real address): + `{"table":"user_data","select":[{"kind":"column","column":"id"},{"kind":"column","column":"created"},{"kind":"column","column":"kycLevel"},{"kind":"column","column":"status"}],"where":{"kind":"leaf","column":"mail","op":"=","value":""},"orderBy":[{"column":"id","direction":"ASC"}],"limit":100}`. + Result is an array of rows — one mail can belong to several `user_data` records. - `asset`: `id`, `name`, `blockchain`, `type`, … — resolve one with a `where` and-tree on `blockchain` + `name`, e.g. `{"kind":"and","children":[{"kind":"leaf","column":"blockchain","op":"=","value":""},{"kind":"leaf","column":"name","op":"=","value":""}]}`. diff --git a/src/subdomains/generic/gs/__tests__/gs.service.spec.ts b/src/subdomains/generic/gs/__tests__/gs.service.spec.ts index 060e873caa..1495773b6f 100644 --- a/src/subdomains/generic/gs/__tests__/gs.service.spec.ts +++ b/src/subdomains/generic/gs/__tests__/gs.service.spec.ts @@ -1,13 +1,19 @@ import { BadRequestException } from '@nestjs/common'; import { createMock } from '@golevelup/ts-jest'; import { DataSource } from 'typeorm'; +import { UserRole } from 'src/shared/auth/user-role.enum'; import { DfxLogger } from 'src/shared/services/dfx-logger'; import { GsService } from '../gs.service'; +import { DbQueryDto } from 'src/subdomains/generic/gs/dto/db-query.dto'; import { assertDebugAllowlistInvariants, + DebugAllowedColumns, + DebugFilterOnlyAllowedOps, + DebugFilterOnlyRestrictedExceptions, DebugQueryAuditPrefix, DebugRestrictedOverlapExceptions, GsRestrictedColumns, + GsRestrictedMarker, } from '../dto/gs.dto'; import { UserDataService } from '../../user/models/user-data/user-data.service'; import { UserService } from '../../user/models/user/user.service'; @@ -202,7 +208,8 @@ describe('GsService', () => { // sensitive. The new allowlist must reject every one. A future migration that adds any // of these to DebugAllowedColumns will fail CI here — that's intentional. it.each([ - // user_data PII + // user_data PII — `mail` is filter-only (WHERE allowed, SELECT/ORDER/GROUP rejected); + // the matrix still asserts SELECT rejection so a future promotion into `columns` fails CI. ['user_data', 'mail'], ['user_data', 'phone'], ['user_data', 'firstname'], @@ -461,10 +468,49 @@ describe('GsService', () => { expect(DebugRestrictedOverlapExceptions['transaction_aml_check']).toEqual(['amlResponsible']); }); - // The exception is scoped to /gs/debug. `/gs/db` masking is driven by GsRestrictedColumns - // and must stay untouched, so a non-SUPER_ADMIN caller there still sees [RESTRICTED]. - it('leaves the /gs/db masking list unchanged', () => { + // The exception is scoped to /gs/debug. `/gs/db` still masks via GsRestrictedColumns — + // exercise the result path so this fails if masking stops applying that list. + it('masks amlResponsible for ADMIN on /gs/db and leaves it visible for SUPER_ADMIN', async () => { expect(GsRestrictedColumns['transaction_aml_check']).toEqual(['amlResponsible', 'comment']); + + const original = 'compliance-officer@dfx.swiss'; + const qb = { + from: jest.fn().mockReturnThis(), + orderBy: jest.fn().mockReturnThis(), + limit: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + andWhere: jest.fn().mockReturnThis(), + select: jest.fn().mockReturnThis(), + leftJoin: jest.fn().mockReturnThis(), + // Fresh row each call — maskRestrictedColumns mutates entries in place. + getRawMany: jest.fn().mockImplementation(async () => [{ id: 1, amlResponsible: original }]), + }; + jest.spyOn(dataSource, 'createQueryBuilder').mockReturnValue(qb as never); + + const query: DbQueryDto = { + table: 'transaction_aml_check', + min: 1, + updatedSince: new Date(0), + sortColumn: 'id', + sorting: 'ASC', + maxLine: 10, + select: ['id', 'amlResponsible'], + where: [], + join: [], + identifier: 'gs-db-amlResponsible-masking', + }; + + const adminResult = await service.getDbData(query, UserRole.ADMIN); + expect(adminResult).toEqual({ + keys: ['id', 'amlResponsible'], + values: [[1, GsRestrictedMarker]], + }); + + const superAdminResult = await service.getDbData(query, UserRole.SUPER_ADMIN); + expect(superAdminResult).toEqual({ + keys: ['id', 'amlResponsible'], + values: [[1, original]], + }); }); // Guard against the exception list growing by accident: every excepted pair must be a @@ -472,6 +518,652 @@ describe('GsService', () => { it('excepts nothing beyond transaction_aml_check.amlResponsible', () => { expect(DebugRestrictedOverlapExceptions).toEqual({ transaction_aml_check: ['amlResponsible'] }); }); + + // Filter-only restricted exceptions are a separate capacity: pin empty so a future + // filter-only overlap cannot slip in without updating this pin. + it('pins DebugFilterOnlyRestrictedExceptions empty (no restricted filter-only column today)', () => { + expect(DebugFilterOnlyRestrictedExceptions).toEqual({}); + }); + }); + + // Filter-only columns: usable solely as a WHERE leaf (`=` only), never + // selectable / orderable / groupable / jsonb-addressable. Driven by looking up + // `user_data.id` from a known `user_data.mail` without ever disclosing the address. + describe('user_data.mail filter-only column', () => { + // --- A. Positive: mail is usable as a filter --- + + it('emits WHERE mail = as case-insensitive LOWER equality with a bound parameter (not inlined)', async () => { + // Matches getUsersByMail (LOWER(mail)); tolerates the caller typing a different case. + // Equality stays equality (exact address required; only letter case is forgiven). + const q = spyQuery([{ id: 42 }]); + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'user@example.com' }, + limit: 10, + }; + + await service.executeDebugQuery(dto, 'tester'); + + const [sql, params] = q.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('LOWER("user_data"."mail") = LOWER($1)'); + expect(sql).not.toContain("'user@example.com'"); + // Plain exact form must not be used for filter-only equality. + expect(sql).not.toContain('"user_data"."mail" = $1'); + expect(params).toEqual(['user@example.com']); + }); + + it('rejects WHERE mail IN (batching multiplies guessing throughput)', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'leaf', + column: 'mail', + op: DebugWhereOp.IN, + value: ['user@example.com', 'other@example.com'], + }, + limit: 10, + }; + + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow( + /Operator 'IN' is not allowed on filter-only column 'mail'/, + ); + }); + + // At most one filter-only predicate per query — OR/AND of several `=` leaves would + // re-enable the multi-candidate batching that removing `IN` was meant to prevent. + it('rejects OR of two mail = leaves (one filter-only predicate per query)', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'or', + children: [ + { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'a@example.com' }, + { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'b@example.com' }, + ], + }, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow( + /Only one filter-only predicate is allowed per query/, + ); + }); + + it('rejects AND of two mail = leaves (one filter-only predicate per query)', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'and', + children: [ + { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'a@example.com' }, + { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'b@example.com' }, + ], + }, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow( + /Only one filter-only predicate is allowed per query/, + ); + }); + + it('rejects deeply nested two mail = leaves (AND of ORs, one filter-only per query)', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'and', + children: [ + { + kind: 'or', + children: [{ kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'a@example.com' }], + }, + { + kind: 'or', + children: [{ kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'b@example.com' }], + }, + ], + }, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow( + /Only one filter-only predicate is allowed per query/, + ); + }); + + it('accepts one mail = leaf combined with several ordinary predicates', async () => { + const q = spyQuery([{ id: 1 }]); + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'and', + children: [ + { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'user@example.com' }, + { kind: 'leaf', column: 'id', op: DebugWhereOp.GT, value: 0 }, + { kind: 'leaf', column: 'status', op: DebugWhereOp.EQ, value: 'Active' }, + { kind: 'leaf', column: 'kycLevel', op: DebugWhereOp.GE, value: 10 }, + ], + }, + limit: 10, + }; + + await service.executeDebugQuery(dto, 'tester'); + + const [sql, params] = q.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('LOWER("user_data"."mail") = LOWER($1)'); + expect(sql).toContain('"user_data"."id" > $2'); + expect(sql).toContain('"user_data"."status" = $3'); + expect(sql).toContain('"user_data"."kycLevel" >= $4'); + expect(params).toEqual(['user@example.com', 0, 'Active', 10]); + }); + + it('still accepts two ordinary predicates on the same ordinary column', async () => { + // The one-predicate cap is filter-only only; ordinary columns keep any number. + const q = spyQuery([{ id: 1 }]); + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'and', + children: [ + { kind: 'leaf', column: 'id', op: DebugWhereOp.GT, value: 0 }, + { kind: 'leaf', column: 'id', op: DebugWhereOp.LT, value: 1000 }, + ], + }, + limit: 10, + }; + + await service.executeDebugQuery(dto, 'tester'); + + const [sql, params] = q.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('"user_data"."id" > $1'); + expect(sql).toContain('"user_data"."id" < $2'); + expect(params).toEqual([0, 1000]); + }); + + it('emits select id where mail = … limit 100 without clamping to a single row', async () => { + // Several user_data rows can share one mail; the multi-match case must remain + // expressible (limit 100, not a forced single-row lookup). + const q = spyQuery([{ id: 1 }, { id: 2 }, { id: 3 }]); + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'user@example.com' }, + limit: 100, + }; + + await service.executeDebugQuery(dto, 'tester'); + + const [sql, params] = q.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('SELECT "user_data"."id" AS "id" FROM "user_data"'); + expect(sql).toContain('LOWER("user_data"."mail") = LOWER($1)'); + // Word-boundary so LIMIT 100 is not mistaken for LIMIT 1 (substring). + expect(sql).toMatch(/LIMIT 100(?:\s|$)/); + expect(sql).not.toMatch(/LIMIT 1(?:\s|$)/); + expect(sql).not.toMatch(/"user_data"\."mail" AS/); + expect(params).toEqual(['user@example.com']); + }); + + it('keeps ordinary allowlisted column equality as plain exact match (not LOWER)', async () => { + // Case-insensitive LOWER emission is filter-only only; ordinary columns stay exact. + const q = spyQuery([{ id: 1 }]); + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { kind: 'leaf', column: 'accountType', op: DebugWhereOp.EQ, value: 'Personal' }, + limit: 10, + }; + + await service.executeDebugQuery(dto, 'tester'); + + const [sql, params] = q.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('"user_data"."accountType" = $1'); + expect(sql).not.toContain('LOWER("user_data"."accountType")'); + expect(params).toEqual(['Personal']); + }); + + // Filter-only equality value validation — missing/null/array values must fail closed + // before any SQL is issued; empty string is a deliberate legitimate exact filter. + it.each([ + { + label: 'value omitted entirely', + value: undefined as unknown, + omitValue: true, + message: /requires a single scalar value/, + }, + { + label: 'value: null', + value: null as unknown, + omitValue: false, + message: /string, number, or boolean/, + }, + { + label: 'value: []', + value: [] as unknown, + omitValue: false, + message: /requires a single scalar value/, + }, + { + label: "value: ['user@example.com']", + value: ['user@example.com'] as unknown, + omitValue: false, + message: /requires a single scalar value/, + }, + ])('rejects filter-only equality when $label', async ({ value, omitValue, message }) => { + const q = spyQuery(); + const leaf: DebugWhereNode = { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ }; + if (!omitValue) leaf.value = value as DebugWhereNode['value']; + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: leaf, + limit: 10, + }; + + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(message); + // Rejection must not reach the database — a thrown error after SQL would still be a defect. + expect(q).not.toHaveBeenCalled(); + }); + + it('binds empty string as an ordinary exact-equality parameter for filter-only mail', async () => { + // Deliberate: an empty address is a legitimate exact filter that simply matches nothing. + const q = spyQuery([]); + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: '' }, + limit: 10, + }; + + await service.executeDebugQuery(dto, 'tester'); + + const [sql, params] = q.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('LOWER("user_data"."mail") = LOWER($1)'); + expect(params).toEqual(['']); + }); + + // --- B. Negative: mail is NOT returnable (security core) --- + + it('rejects SELECT of mail as a column', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'mail' }], + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(/Column 'mail' is not allowed/); + }); + + it('rejects SELECT aggregate over mail (no leak via aggregation)', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'aggregate', aggregate: DebugAggregate.COUNT, column: 'mail' }], + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(/Column 'mail' is not allowed/); + }); + + it('rejects SELECT jsonb path on mail', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'jsonb', column: 'mail', jsonbPath: 'x' }], + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(/Column 'mail' is not allowed/); + }); + + it('rejects SELECT of mail under an alias (alias does not launder)', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'mail', as: 'email' }], + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(/Column 'mail' is not allowed/); + }); + + it('rejects ORDER BY mail', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + orderBy: [{ column: 'mail' }], + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(/neither an allowed column/); + }); + + it('rejects GROUP BY mail', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + groupBy: ['mail'], + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(/neither an allowed column/); + }); + + it('resolves GROUP BY / ORDER BY of alias mail to the aliased id, not the filter-only column', async () => { + // Register an ordinary allowlisted column under the alias name `mail` so SELECT + // succeeds and the alias is recorded. GROUP BY must emit the select ordinal; + // ORDER BY must emit the quoted alias. Neither may resolve to the physical + // filter-only column `"user_data"."mail"`. + // jest.spyOn reuses the same mock for both queries; clear so calls[0] is this query's SQL. + const q = spyQuery(); + const groupDto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id', as: 'mail' }], + groupBy: ['mail'], + limit: 10, + }; + await service.executeDebugQuery(groupDto, 'tester'); + const groupSql = q.mock.calls[0][0] as string; + expect(groupSql).toContain('GROUP BY 1'); + expect(groupSql).not.toContain('GROUP BY "mail"'); + expect(groupSql).not.toContain('"user_data"."mail"'); + + q.mockClear(); + const orderDto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id', as: 'mail' }], + orderBy: [{ column: 'mail' }], + limit: 10, + }; + await service.executeDebugQuery(orderDto, 'tester'); + const orderSql = q.mock.calls[0][0] as string; + expect(orderSql).toContain('ORDER BY "mail"'); + expect(orderSql).not.toContain('"user_data"."mail"'); + }); + + // --- C. Operator gating --- + + // Every op outside DebugFilterOnlyAllowedOps must be rejected on filter-only columns + // so range/pattern operators cannot oracle-reconstruct the value. IN is also rejected + // (batching multiplies guessing throughput) — see the dedicated IN rejection test above. + it.each( + [ + { op: DebugWhereOp.NE, value: 'user@example.com' as string | string[] | undefined }, + { op: DebugWhereOp.LT, value: 'user@example.com' }, + { op: DebugWhereOp.LE, value: 'user@example.com' }, + { op: DebugWhereOp.GT, value: 'user@example.com' }, + { op: DebugWhereOp.GE, value: 'user@example.com' }, + { op: DebugWhereOp.IN, value: ['user@example.com'] }, + { op: DebugWhereOp.NOT_IN, value: ['user@example.com'] }, + { op: DebugWhereOp.LIKE, value: '%@example.com' }, + { op: DebugWhereOp.ILIKE, value: '%@example.com' }, + { op: DebugWhereOp.IS_NULL, value: undefined }, + { op: DebugWhereOp.IS_NOT_NULL, value: undefined }, + ].filter(({ op }) => !DebugFilterOnlyAllowedOps.includes(op)), + )('rejects WHERE mail with disallowed operator $op', async ({ op, value }) => { + const leaf: DebugWhereNode = { kind: 'leaf', column: 'mail', op }; + if (value !== undefined) leaf.value = value; + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: leaf, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow( + new RegExp( + `Operator '${op.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}' is not allowed on filter-only column 'mail'`, + ), + ); + }); + + it('still allows non-equality operators on ordinary allowlisted user_data columns', async () => { + // Gating is per-column, not a global op lockdown. + const cases: Array<{ op: DebugWhereOp; value?: string | number | number[] }> = [ + { op: DebugWhereOp.NE, value: 1 }, + { op: DebugWhereOp.LT, value: 10 }, + { op: DebugWhereOp.LE, value: 10 }, + { op: DebugWhereOp.GT, value: 0 }, + { op: DebugWhereOp.GE, value: 0 }, + { op: DebugWhereOp.LIKE, value: '1%' }, + { op: DebugWhereOp.ILIKE, value: '1%' }, + { op: DebugWhereOp.IS_NULL }, + { op: DebugWhereOp.IS_NOT_NULL }, + ]; + for (const { op, value } of cases) { + const q = spyQuery(); + // jest.spyOn reuses the same mock across iterations; clear so calls[0] is this op's SQL. + q.mockClear(); + const leaf: DebugWhereNode = { kind: 'leaf', column: 'id', op }; + if (value !== undefined) leaf.value = value; + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: leaf, + limit: 10, + }; + await service.executeDebugQuery(dto, 'tester'); + expect(q).toHaveBeenCalled(); + const sql = q.mock.calls[0][0] as string; + expect(sql).toContain(`"user_data"."id" ${op}`); + } + }); + + // --- D. NOT must not launder a filter-only leaf --- + + it('rejects NOT (mail = …) — would bypass the operator gate as mail != …', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'not', + child: { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'user@example.com' }, + }, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow( + /Filter-only column 'mail' cannot be used inside a NOT/, + ); + }); + + it('rejects nested/deeper NOT over mail (AND wrapping NOT)', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'and', + children: [ + { kind: 'leaf', column: 'id', op: DebugWhereOp.EQ, value: 1 }, + { + kind: 'not', + child: { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'user@example.com' }, + }, + ], + }, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow( + /Filter-only column 'mail' cannot be used inside a NOT/, + ); + }); + + it('rejects double NOT over mail (double negation is not cancelled)', async () => { + // Any negation above a filter-only leaf is refused — NOT(NOT(...)) does not cancel. + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'not', + child: { + kind: 'not', + child: { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'user@example.com' }, + }, + }, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow( + /Filter-only column 'mail' cannot be used inside a NOT/, + ); + }); + + // `negated` is propagated into and/or children (not only direct NOT→leaf). Without these + // cases a regression that stopped propagating through a boolean node would stay green. + it('rejects NOT ( AND [ mail = x, id = 1 ] ) — negated flag propagates into AND', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'not', + child: { + kind: 'and', + children: [ + { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'user@example.com' }, + { kind: 'leaf', column: 'id', op: DebugWhereOp.EQ, value: 1 }, + ], + }, + }, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow( + /Filter-only column 'mail' cannot be used inside a NOT/, + ); + }); + + it('rejects NOT ( OR [ mail = x, id = 1 ] ) — negated flag propagates into OR', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'not', + child: { + kind: 'or', + children: [ + { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'user@example.com' }, + { kind: 'leaf', column: 'id', op: DebugWhereOp.EQ, value: 1 }, + ], + }, + }, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow( + /Filter-only column 'mail' cannot be used inside a NOT/, + ); + }); + + it('rejects NOT ( AND [ OR [ mail = x ] ] ) — deeper nesting still propagates', async () => { + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'not', + child: { + kind: 'and', + children: [ + { + kind: 'or', + children: [{ kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'user@example.com' }], + }, + ], + }, + }, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(BadRequestException); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow( + /Filter-only column 'mail' cannot be used inside a NOT/, + ); + }); + + it('accepts AND [ NOT (id = 1), mail = x ] — filter-only leaf not under the negation', async () => { + // Positive control: pins propagation semantics rather than a blanket refusal of + // any tree that contains both NOT and a filter-only leaf. + const q = spyQuery([{ id: 2 }]); + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'and', + children: [ + { + kind: 'not', + child: { kind: 'leaf', column: 'id', op: DebugWhereOp.EQ, value: 1 }, + }, + { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'user@example.com' }, + ], + }, + limit: 10, + }; + + await service.executeDebugQuery(dto, 'tester'); + + const [sql, params] = q.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('NOT "user_data"."id" = $1'); + expect(sql).toContain('LOWER("user_data"."mail") = LOWER($2)'); + expect(params).toEqual([1, 'user@example.com']); + }); + + it('still allows NOT over an ordinary allowlisted column', async () => { + const q = spyQuery([{ id: 1 }]); + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'not', + child: { kind: 'leaf', column: 'id', op: DebugWhereOp.EQ, value: 1 }, + }, + limit: 10, + }; + + await service.executeDebugQuery(dto, 'tester'); + + const [sql, params] = q.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('NOT "user_data"."id" = $1'); + expect(params).toEqual([1]); + }); + + // --- E. Pinning + audit --- + + it('pins filterOnlyColumns to exactly [mail] and keeps mail out of columns', () => { + expect(DebugAllowedColumns['user_data'].filterOnlyColumns).toEqual(['mail']); + expect(DebugAllowedColumns['user_data'].columns).not.toContain('mail'); + }); + + it('pins DebugFilterOnlyAllowedOps to equality only', () => { + expect(DebugFilterOnlyAllowedOps).toEqual([DebugWhereOp.EQ]); + }); + + it('redacts a realistic mail filter value in the audit log', async () => { + const verboseSpy = jest.spyOn(DfxLogger.prototype, 'verbose').mockImplementation(() => undefined); + spyQuery([{ id: 1 }]); + const address = 'user@example.com'; + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: address }, + limit: 10, + }; + + await service.executeDebugQuery(dto, '0xtester'); + + const auditLine = verboseSpy.mock.calls + .map((c) => String(c[0])) + .find((l) => l.startsWith('Debug-query by 0xtester:')); + expect(auditLine).toBeDefined(); + expect(auditLine).not.toContain(address); + expect(auditLine).toContain('"value":""'); + expect(auditLine).toContain('"column":"mail"'); + verboseSpy.mockRestore(); + }); }); // The assertions above only prove today's constants are consistent with each other. They @@ -482,7 +1174,7 @@ describe('GsService', () => { const spec = (...columns: string[]) => ({ columns }); it('throws on an overlap that is not registered as an exception', () => { - expect(() => assertDebugAllowlistInvariants({ t: ['secret'] }, { t: spec('id', 'secret') }, {})).toThrow( + expect(() => assertDebugAllowlistInvariants({ t: ['secret'] }, { t: spec('id', 'secret') }, {}, {})).toThrow( /contains 'secret' which is in GsRestrictedColumns/, ); }); @@ -490,35 +1182,35 @@ describe('GsService', () => { // The core of the finding: excepting one column must not amnesty its table. it('excepting one column does not amnesty a second overlap in the same table', () => { expect(() => - assertDebugAllowlistInvariants({ t: ['ok', 'secret'] }, { t: spec('ok', 'secret') }, { t: ['ok'] }), + assertDebugAllowlistInvariants({ t: ['ok', 'secret'] }, { t: spec('ok', 'secret') }, { t: ['ok'] }, {}), ).toThrow(/contains 'secret' which is in GsRestrictedColumns/); }); it('passes for a registered overlap', () => { expect(() => - assertDebugAllowlistInvariants({ t: ['ok'] }, { t: spec('id', 'ok') }, { t: ['ok'] }), + assertDebugAllowlistInvariants({ t: ['ok'] }, { t: spec('id', 'ok') }, { t: ['ok'] }, {}), ).not.toThrow(); }); it('passes when a restricted column is simply absent from the allowlist', () => { - expect(() => assertDebugAllowlistInvariants({ t: ['secret'] }, { t: spec('id') }, {})).not.toThrow(); + expect(() => assertDebugAllowlistInvariants({ t: ['secret'] }, { t: spec('id') }, {}, {})).not.toThrow(); }); it('throws on a stale exception whose column left GsRestrictedColumns', () => { - expect(() => assertDebugAllowlistInvariants({ t: [] }, { t: spec('ok') }, { t: ['ok'] })).toThrow( + expect(() => assertDebugAllowlistInvariants({ t: [] }, { t: spec('ok') }, { t: ['ok'] }, {})).toThrow( /not in GsRestrictedColumns/, ); }); it('throws on a stale exception whose column left DebugAllowedColumns', () => { - expect(() => assertDebugAllowlistInvariants({ t: ['ok'] }, { t: spec('id') }, { t: ['ok'] })).toThrow( - /not in DebugAllowedColumns/, + expect(() => assertDebugAllowlistInvariants({ t: ['ok'] }, { t: spec('id') }, { t: ['ok'] }, {})).toThrow( + /not in DebugAllowedColumns\['t'\]\.columns/, ); }); it('throws on a stale exception for a table that is not debuggable at all', () => { - expect(() => assertDebugAllowlistInvariants({ t: ['ok'] }, {}, { t: ['ok'] })).toThrow( - /not in DebugAllowedColumns/, + expect(() => assertDebugAllowlistInvariants({ t: ['ok'] }, {}, { t: ['ok'] }, {})).toThrow( + /not in DebugAllowedColumns\['t'\]\.columns/, ); }); @@ -527,12 +1219,217 @@ describe('GsService', () => { // prototype instead). Without the `Object.hasOwn` guards, `allowedColumns['__proto__']` // resolves to `Object.prototype` and `.columns.includes` would TypeError into a 500. it('does not resolve prototype-chain keys through Object.prototype', () => { - expect(() => assertDebugAllowlistInvariants({ ['__proto__']: ['ok'] }, {}, {})).not.toThrow(); - expect(() => assertDebugAllowlistInvariants({ ['constructor']: ['ok'] }, {}, {})).not.toThrow(); - expect(() => assertDebugAllowlistInvariants({}, {}, { ['constructor']: ['ok'] })).toThrow( + expect(() => assertDebugAllowlistInvariants({ ['__proto__']: ['ok'] }, {}, {}, {})).not.toThrow(); + expect(() => assertDebugAllowlistInvariants({ ['constructor']: ['ok'] }, {}, {}, {})).not.toThrow(); + expect(() => assertDebugAllowlistInvariants({}, {}, { ['constructor']: ['ok'] }, {})).toThrow( /not in GsRestrictedColumns/, ); }); + + // --- D. filterOnlyColumns invariants (synthetic fixtures) --- + + it('accepts an empty filterOnlyColumns list as a valid configuration', () => { + expect(() => + assertDebugAllowlistInvariants({}, { t: { columns: ['id'], filterOnlyColumns: [] } }, {}, {}), + ).not.toThrow(); + }); + + it('throws when the same column appears in both columns and filterOnlyColumns', () => { + expect(() => + assertDebugAllowlistInvariants( + {}, + { t: { columns: ['id', 'secret'], filterOnlyColumns: ['secret'] } }, + {}, + {}, + ), + ).toThrow(/filterOnlyColumns contains 'secret' which is also in columns/); + }); + + it('throws when the same column appears in both jsonbColumns and filterOnlyColumns', () => { + expect(() => + assertDebugAllowlistInvariants( + {}, + { t: { columns: ['id'], jsonbColumns: ['payload'], filterOnlyColumns: ['payload'] } }, + {}, + {}, + ), + ).toThrow(/filterOnlyColumns contains 'payload' which is also in jsonbColumns/); + }); + + it('throws on a duplicate entry inside filterOnlyColumns', () => { + expect(() => + assertDebugAllowlistInvariants({}, { t: { columns: ['id'], filterOnlyColumns: ['mail', 'mail'] } }, {}, {}), + ).toThrow(/filterOnlyColumns has duplicate entry 'mail'/); + }); + + it('throws when a filter-only column is also in GsRestrictedColumns unless exactly excepted', () => { + expect(() => + assertDebugAllowlistInvariants( + { t: ['secret'] }, + { t: { columns: ['id'], filterOnlyColumns: ['secret'] } }, + {}, + {}, + ), + ).toThrow(/filterOnlyColumns contains 'secret' which is in GsRestrictedColumns/); + + // Filter-only capacity requires the filter-only exceptions map — not the selectable one. + expect(() => + assertDebugAllowlistInvariants( + { t: ['secret'] }, + { t: { columns: ['id'], filterOnlyColumns: ['secret'] } }, + {}, + { t: ['secret'] }, + ), + ).not.toThrow(); + }); + + // --- E. Capacity-specific exceptions (selectable vs filter-only) --- + + // Core hole this pin closes: a filter-only exception must not authorise selectable + // access after someone moves the column into `columns`. + it('does not let a filter-only exception authorise selectable access (capacity upgrade)', () => { + // filter-only + matching filter-only exception: OK + expect(() => + assertDebugAllowlistInvariants( + { t: ['secret'] }, + { t: { columns: ['id'], filterOnlyColumns: ['secret'] } }, + {}, + { t: ['secret'] }, + ), + ).not.toThrow(); + + // Same exception, column moved into `columns` → fails (selectable needs its own map; + // the leftover filter-only exception is also stale). + expect(() => + assertDebugAllowlistInvariants( + { t: ['secret'] }, + { t: { columns: ['id', 'secret'] } }, + {}, + { t: ['secret'] }, + ), + ).toThrow(/contains 'secret' which is in GsRestrictedColumns/); + }); + + it('passes for a selectable exception when the column is in columns', () => { + // Mirrors transaction_aml_check.amlResponsible: restricted + columns + selectable map. + expect(() => + assertDebugAllowlistInvariants( + { t: ['secret'] }, + { t: { columns: ['id', 'secret'] } }, + { t: ['secret'] }, + {}, + ), + ).not.toThrow(); + }); + + it('throws when a selectable exception covers a column that is only filter-only', () => { + // Filter-only exception keeps the restricted filter-only column legal; the selectable + // exception for the same pair is stale because the column is not in `columns`. + expect(() => + assertDebugAllowlistInvariants( + { t: ['secret'] }, + { t: { columns: ['id'], filterOnlyColumns: ['secret'] } }, + { t: ['secret'] }, + { t: ['secret'] }, + ), + ).toThrow(/not in DebugAllowedColumns\['t'\]\.columns/); + }); + + it('throws when a filter-only exception covers a column that is only in columns', () => { + // Selectable exception keeps the restricted selectable column legal; the filter-only + // exception for the same pair is then either dual-registered or stale for capacity. + // With only the filter-only map, the missing selectable approval fails first — either + // way the wrong-capacity configuration is rejected. + expect(() => + assertDebugAllowlistInvariants( + { t: ['secret'] }, + { t: { columns: ['id', 'secret'] } }, + {}, + { t: ['secret'] }, + ), + ).toThrow(/contains 'secret' which is in GsRestrictedColumns/); + + // Pure capacity-stale path: selectable approval is present so loop 1 passes; dual + // registration of the same pair is rejected (approval must be unambiguous). + expect(() => + assertDebugAllowlistInvariants( + { t: ['secret'] }, + { t: { columns: ['id', 'secret'] } }, + { t: ['secret'] }, + { t: ['secret'] }, + ), + ).toThrow(/registered in both/); + }); + + it('throws when the same pair is registered in both exception maps', () => { + // Dual registration with a selectable column: selectable membership passes, dual fails. + expect(() => + assertDebugAllowlistInvariants( + { t: ['secret'] }, + { t: { columns: ['id', 'secret'] } }, + { t: ['secret'] }, + { t: ['secret'] }, + ), + ).toThrow(/registered in both/); + }); + + it('throws on a stale filter-only exception whose column left filterOnlyColumns', () => { + expect(() => + assertDebugAllowlistInvariants({ t: ['secret'] }, { t: { columns: ['id'] } }, {}, { t: ['secret'] }), + ).toThrow(/not in DebugAllowedColumns\['t'\]\.filterOnlyColumns/); + }); + + it('throws on a stale filter-only exception whose column left GsRestrictedColumns', () => { + expect(() => + assertDebugAllowlistInvariants( + { t: [] }, + { t: { columns: ['id'], filterOnlyColumns: ['secret'] } }, + {}, + { t: ['secret'] }, + ), + ).toThrow(/not in GsRestrictedColumns/); + }); + + it('does not resolve prototype-chain keys through Object.prototype (filter-only path)', () => { + // Computed keys so these are real own properties. The filterOnlyColumns loop and the + // restricted-overlap check both use Object.hasOwn / Object.entries — prototype keys + // must not resolve via Object.prototype or TypeError into a 500. + expect(() => + assertDebugAllowlistInvariants( + { ['__proto__']: ['ok'] }, + { t: { columns: ['id'], filterOnlyColumns: ['mail'] } }, + {}, + {}, + ), + ).not.toThrow(); + expect(() => + assertDebugAllowlistInvariants( + { ['constructor']: ['ok'] }, + { t: { columns: ['id'], filterOnlyColumns: ['mail'] } }, + {}, + {}, + ), + ).not.toThrow(); + expect(() => + assertDebugAllowlistInvariants({}, { ['__proto__']: { columns: [], filterOnlyColumns: ['x'] } }, {}, {}), + ).not.toThrow(); + expect(() => + assertDebugAllowlistInvariants( + {}, + { t: { columns: ['id'], filterOnlyColumns: ['mail'] } }, + { ['constructor']: ['ok'] }, + {}, + ), + ).toThrow(/not in GsRestrictedColumns/); + expect(() => + assertDebugAllowlistInvariants( + {}, + { t: { columns: ['id'], filterOnlyColumns: ['mail'] } }, + {}, + { ['constructor']: ['ok'] }, + ), + ).toThrow(/not in GsRestrictedColumns/); + }); }); it('allows multiple safe columns in a single SELECT', async () => { @@ -711,23 +1608,38 @@ describe('GsService', () => { }); it('rejects = on a disallowed column (PII)', async () => { + // `phone` is fully blocked (not filter-only). `mail` is filter-only — its WHERE path is + // covered by the dedicated positive tests under `user_data.mail filter-only column`. const dto: DebugQueryDto = { table: 'user_data', select: [{ kind: 'column', column: 'id' }], - where: { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: 'a@b.c' }, + where: { kind: 'leaf', column: 'phone', op: DebugWhereOp.EQ, value: '+41000000000' }, limit: 10, }; await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(/Column .*not allowed/); }); - it('rejects an object/array as a scalar value', async () => { + // Pins both halves of the scalar check: objects fall through to assertDebugScalarValue, + // arrays are refused earlier by the Array.isArray guard on single-scalar ops. + it.each([ + { + label: 'object', + value: { a: 1 } as never, + message: /string, number, or boolean/, + }, + { + label: 'array', + value: [1, 2] as never, + message: /requires a single scalar value/, + }, + ])('rejects an $label as a scalar value', async ({ value, message }) => { const dto = { table: 'asset', select: [{ kind: 'column' as const, column: 'id' }], - where: { kind: 'leaf' as const, column: 'id', op: DebugWhereOp.EQ, value: { a: 1 } as never }, + where: { kind: 'leaf' as const, column: 'id', op: DebugWhereOp.EQ, value }, limit: 10, }; - await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(/string, number, or boolean/); + await expect(service.executeDebugQuery(dto, 'tester')).rejects.toThrow(message); }); }); @@ -2019,10 +2931,19 @@ describe('GsService', () => { verboseSpy.mockRestore(); }); - it('writes an info-level log line when the query throws', async () => { + // Intentional change of guarantee: the failure path used to log e.message verbatim. + // Postgres (and some drivers) echo bound parameter values in error messages, so + // that undid WHERE-value redaction. Failures are now logged by SQLSTATE / severity / + // routine only — never by message. + it('writes an info-level failure log with value-free diagnostics (code, not message)', async () => { const infoSpy = jest.spyOn(DfxLogger.prototype, 'info').mockImplementation(() => undefined); jest.spyOn(dataSource, 'query').mockImplementation(async () => { - throw new Error('boom'); + const err = Object.assign(new Error('boom — must not appear in logs'), { + code: '22P02', + severity: 'ERROR', + routine: 'pg_atoi', + }); + throw err; }); const dto: DebugQueryDto = { table: 'asset', @@ -2031,8 +2952,74 @@ describe('GsService', () => { }; await expect(service.executeDebugQuery(dto, '0xtester')).rejects.toThrow(/Query execution failed/); const lines = infoSpy.mock.calls.map((c) => String(c[0])); - expect(lines.some((l) => l.startsWith('Debug-query by 0xtester failed:'))).toBe(true); - expect(lines.some((l) => l.includes('boom'))).toBe(true); + const failLine = lines.find((l) => l.startsWith('Debug-query by 0xtester failed:')); + expect(failLine).toBeDefined(); + expect(failLine).toContain('code=22P02'); + expect(failLine).toContain('severity=ERROR'); + expect(failLine).toContain('routine=pg_atoi'); + expect(failLine).not.toContain('boom'); + infoSpy.mockRestore(); + }); + + it('does not log addresses echoed in database error messages (redaction guarantee)', async () => { + // Regression: filtering mail = 'user@example.com' AND id = 'user@example.com' makes + // Postgres reject the id parameter with a message that embeds the address. That + // message must never reach the log; only the SQLSTATE (and other value-free fields). + const leakedAddress = 'user@example.com'; + const infoSpy = jest.spyOn(DfxLogger.prototype, 'info').mockImplementation(() => undefined); + jest.spyOn(dataSource, 'query').mockImplementation(async () => { + const err = Object.assign(new Error(`invalid input syntax for type integer: "${leakedAddress}"`), { + code: '22P02', + severity: 'ERROR', + routine: 'pg_atoi', + }); + throw err; + }); + const dto: DebugQueryDto = { + table: 'user_data', + select: [{ kind: 'column', column: 'id' }], + where: { + kind: 'and', + children: [ + { kind: 'leaf', column: 'mail', op: DebugWhereOp.EQ, value: leakedAddress }, + { kind: 'leaf', column: 'id', op: DebugWhereOp.EQ, value: leakedAddress }, + ], + }, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, '0xtester')).rejects.toThrow(/Query execution failed/); + const failCalls = infoSpy.mock.calls.filter((c) => String(c[0]).startsWith('Debug-query by 0xtester failed:')); + expect(failCalls.length).toBeGreaterThan(0); + // A regression to `logger.info(diagnostics, e)` would pass the error as a second + // argument whose stack embeds the address; assert arity so that stays red. + for (const call of failCalls) { + expect(call[1]).toBeUndefined(); + } + const lines = infoSpy.mock.calls.map((c) => String(c[0])); + const failLine = lines.find((l) => l.startsWith('Debug-query by 0xtester failed:')); + expect(failLine).toBeDefined(); + expect(failLine).toContain('code=22P02'); + expect(failLine).not.toContain(leakedAddress); + expect(lines.every((l) => !l.includes(leakedAddress))).toBe(true); + infoSpy.mockRestore(); + }); + + it('logs a stable code placeholder when the driver error has no code', async () => { + const infoSpy = jest.spyOn(DfxLogger.prototype, 'info').mockImplementation(() => undefined); + jest.spyOn(dataSource, 'query').mockImplementation(async () => { + throw new Error('connection reset — must not appear in logs'); + }); + const dto: DebugQueryDto = { + table: 'asset', + select: [{ kind: 'column', column: 'id' }], + limit: 10, + }; + await expect(service.executeDebugQuery(dto, '0xtester')).rejects.toThrow(/Query execution failed/); + const lines = infoSpy.mock.calls.map((c) => String(c[0])); + const failLine = lines.find((l) => l.startsWith('Debug-query by 0xtester failed:')); + expect(failLine).toBeDefined(); + expect(failLine).toContain('code='); + expect(failLine).not.toContain('connection reset'); infoSpy.mockRestore(); }); @@ -2056,6 +3043,30 @@ describe('GsService', () => { verboseSpy.mockRestore(); }); + it('still writes the audit log for an unknown-table probe (before allowlist rejection)', async () => { + // Audit must fire before the table allowlist check so probes of non-allowlisted + // tables are attributable. Values stay redacted as in every other audit path. + const verboseSpy = jest.spyOn(DfxLogger.prototype, 'verbose').mockImplementation(() => undefined); + const secret = 'super-secret-value'; + const dto = { + table: 'pg_catalog_pg_roles', + select: [{ kind: 'column' as const, column: 'id' }], + where: { kind: 'leaf' as const, column: 'rolname', op: DebugWhereOp.EQ, value: secret }, + limit: 10, + }; + await expect(service.executeDebugQuery(dto, '0xprobe')).rejects.toThrow( + /Table 'pg_catalog_pg_roles' is not allowed/, + ); + const auditLine = verboseSpy.mock.calls + .map((c) => String(c[0])) + .find((l) => l.startsWith('Debug-query by 0xprobe:')); + expect(auditLine).toBeDefined(); + expect(auditLine).toContain('"table":"pg_catalog_pg_roles"'); + expect(auditLine).not.toContain(secret); + expect(auditLine).toContain('"value":""'); + verboseSpy.mockRestore(); + }); + it('audit JSON payload contains the table name', async () => { const verboseSpy = jest.spyOn(DfxLogger.prototype, 'verbose').mockImplementation(() => undefined); spyQuery(); @@ -2488,6 +3499,11 @@ describe('DebugQueryDto - ValidationPipe layer', () => { ['limit too small', { table: 'asset', select: [{ kind: 'column', column: 'id' }], limit: 0 }, 'min'], ['limit too large', { table: 'asset', select: [{ kind: 'column', column: 'id' }], limit: 10001 }, 'max'], ['offset negative', { table: 'asset', select: [{ kind: 'column', column: 'id' }], limit: 10, offset: -1 }, 'min'], + [ + 'offset too large', + { table: 'asset', select: [{ kind: 'column', column: 'id' }], limit: 10, offset: 1000001 }, + 'max', + ], ])('rejects %s', async (_label, payload, expectedConstraint) => { const errors = await validateDto(payload); expect(errors.length).toBeGreaterThan(0); diff --git a/src/subdomains/generic/gs/dto/debug-query.dto.ts b/src/subdomains/generic/gs/dto/debug-query.dto.ts index 694d9144b9..9d8793be2f 100644 --- a/src/subdomains/generic/gs/dto/debug-query.dto.ts +++ b/src/subdomains/generic/gs/dto/debug-query.dto.ts @@ -20,9 +20,10 @@ import { // Structured /gs/debug DTO. // -// The endpoint accepts a JSON description of a query and emits SQL via TypeORM QueryBuilder -// with parameter binding. No raw SQL ever crosses the wire; identifiers are pulled from -// DebugAllowedColumns in gs.dto.ts, and values flow exclusively through bound parameters. +// The endpoint accepts a JSON description of a query and emits SQL manually with parameter +// binding, then executes it via `dataSource.query`. No raw SQL ever crosses the wire; +// identifiers are pulled from DebugAllowedColumns in gs.dto.ts, and values flow exclusively +// through bound parameters. // // This is deliberately a narrow surface — every shape the executor can produce is enumerated // here. To add functionality (CASE, window funcs, OR-with-NOT-NULL, …) extend the schema diff --git a/src/subdomains/generic/gs/dto/gs.dto.ts b/src/subdomains/generic/gs/dto/gs.dto.ts index 660f1052a6..fd4fec3c49 100644 --- a/src/subdomains/generic/gs/dto/gs.dto.ts +++ b/src/subdomains/generic/gs/dto/gs.dto.ts @@ -1,3 +1,5 @@ +import { DebugWhereOp } from 'src/subdomains/generic/gs/dto/debug-query.dto'; + export const GsRestrictedMarker = '[RESTRICTED]'; // db endpoint @@ -22,9 +24,9 @@ export const DebugMaxResults = 10000; // --- Structured /gs/debug allowlist --- // // The /gs/debug endpoint accepts a JSON request describing the query (table + select + where -// + group/order/limit) and emits SQL via TypeORM with parameter binding. No raw SQL is ever -// accepted, parsed, or interpolated — identifiers come exclusively from this allowlist and -// values flow through bound parameters. +// + group/order/limit) and emits hand-built SQL with bound parameters via `dataSource.query`. +// No raw SQL is ever accepted, parsed, or interpolated — identifiers come exclusively from +// this allowlist and values flow through bound parameters. // // Update on every migration: adding / renaming / removing a column on a table that appears // below requires editing this allowlist. Adding a new debuggable table requires a new entry. @@ -32,8 +34,10 @@ export const DebugMaxResults = 10000; // // Conservative inclusion rules — exclude these categories of columns even when present on // the entity: -// - PII: names, addresses, phone, mail, birthday, nationality/country FKs on user_data, -// organization PII, IBANs, BICs, account numbers. +// - PII (from selectable / result columns): names, addresses, phone, mail, birthday, +// nationality/country FKs on user_data, organization PII, IBANs, BICs, account numbers. +// Explicitly reviewed **filter-only** exceptions may appear in `filterOnlyColumns` only +// (currently `user_data.mail`) — WHERE `=` lookup, never returned in the result set. // - Secrets: apiKey, apiKeyCT, apiUrl, totpSecret, signature, kycHash, uid, pdfUrl. // - Free-form text: label, internalAmlNote, txInfo, raw, data, message (except log.message // which is the whole point of the endpoint). `comment` is a deliberate exception: it is @@ -51,8 +55,34 @@ export interface DebugTableSpec { // Subset of `columns` where the structured `jsonbPath` selector is allowed. The endpoint // emits `(col)::jsonb -> 'a' -> 'b' ->> 'c'` for these; segment names are validated by regex. jsonbColumns?: string[]; + // Columns usable ONLY as a WHERE-leaf column. Never selectable, never orderable, never + // groupable, never usable with a `jsonbPath`. Restricted to the equality operator + // (`DebugFilterOnlyAllowedOps`). At most one filter-only predicate is allowed per query + // (anywhere in the WHERE tree) so multi-candidate batching cannot be reintroduced via + // OR/AND of several `=` leaves. Intended for looking a record up by a value the caller + // already knows, without the endpoint ever disclosing that value. MUST be disjoint from + // `columns` and `jsonbColumns` (enforced by `assertDebugAllowlistInvariants`). + // Expected to be **text** columns: equality is emitted case-insensitively as + // `LOWER(col) = LOWER($n)` so the lookup matches the application's own case-insensitive + // address identity (`getUsersByMail` via `LOWER(mail)`) and tolerates the caller typing + // a different case than stored. A non-text filter-only column (e.g. integer) would fail + // at query time (`function lower(integer) does not exist`). No runtime type check — keep + // entries text-only when extending this list. For `user_data.mail`, `LOWER(mail)` is + // backed by a functional index (non-unique while case-collision duplicates remain); that + // is not a general guarantee for every future filter-only column. + filterOnlyColumns?: string[]; } +// Operators a filter-only column may appear with in a WHERE leaf. Ordering/range/pattern +// operators (`<`, `<=`, `>`, `>=`, `!=`, `LIKE`, `ILIKE`, `IS NULL`, `IS NOT NULL`) would turn +// the endpoint into an oracle — a caller could binary-search or pattern-match a secret value +// character by character and reconstruct it without ever selecting it. `=` requires knowing +// the exact value up front, which is the intended use case. Multi-candidate batching is +// prevented by the one-filter-only-predicate-per-query rule in the emitter (not by excluding +// `IN` alone — OR of several `=` leaves would otherwise re-enable the same batching). `IN` +// remains disallowed because a multi-value list has no legitimate filter-only use. +export const DebugFilterOnlyAllowedOps: DebugWhereOp[] = [DebugWhereOp.EQ]; + export const DebugAllowedColumns: Record = { account_merge: { columns: [ @@ -1432,8 +1462,11 @@ export const DebugAllowedColumns: Record = { ], }, user_data: { - // No PII columns. countryId / nationalityId / organizationId / verifiedCountryId / - // accountOpenerId / organizationCountryId all blocked (link to PII tables). + // No PII in selectable / result columns. countryId / nationalityId / organizationId / + // verifiedCountryId / accountOpenerId / organizationCountryId all blocked (link to PII + // tables). Explicit filter-only exception: `mail` (not in `columns`) — support needs to + // resolve a customer's `userData.id` from a mail address they already have, without the + // endpoint ever returning the address. See `filterOnlyColumns` below. columns: [ 'id', 'created', @@ -1489,6 +1522,7 @@ export const DebugAllowedColumns: Record = { 'tradeApprovalDate', 'walletId', ], + filterOnlyColumns: ['mail'], }, user_data_relation: { columns: ['id', 'created', 'updated', 'accountId', 'relatedAccountId', 'relation', 'signatory'], @@ -1570,11 +1604,15 @@ export const DebugAllowedColumns: Record = { }, }; -// Columns that are deliberately reachable on `/gs/debug` even though `/gs/db` masks them for -// every role below SUPER_ADMIN. Each entry is an explicit, reviewed decision to accept that a -// DEBUG-role caller sees the real value on the structured endpoint — NOT a general relaxation: -// `/gs/db` masking is unchanged, and every overlap that is not listed here still aborts module -// load below. +// Columns that are deliberately **selectable** on `/gs/debug` even though `/gs/db` masks them +// for every role below SUPER_ADMIN. Each entry is an explicit, reviewed decision to accept that a +// DEBUG-role caller sees the real value in the result set — NOT a general relaxation: +// `/gs/db` masking is unchanged, and every selectable overlap that is not listed here still aborts +// module load below. This map approves **`columns` (selectable) access only**. Filter-only +// (`filterOnlyColumns`) overlaps use `DebugFilterOnlyRestrictedExceptions` instead — the two +// capacities do not share exceptions, so moving a column between `columns` and `filterOnlyColumns` +// fails module load until the approval is moved too (a visible, reviewable step that prevents a +// filter-only exception from silently authorising full disclosure). // // Add an entry ONLY when all of the following hold, and record the reasoning in the comment: // - the value is needed to answer an operational/forensic question the endpoint exists for, @@ -1590,58 +1628,151 @@ export const DebugRestrictedOverlapExceptions: Record = { transaction_aml_check: ['amlResponsible'], }; +// Columns that are deliberately reachable as **filter-only** (`filterOnlyColumns`) on `/gs/debug` +// even though `/gs/db` masks them for every role below SUPER_ADMIN. Same review bar as +// `DebugRestrictedOverlapExceptions`, but this map approves equality-only WHERE lookup — never +// selection. Keep the two maps disjoint: an entry in both for the same pair is an error (approval +// must be unambiguous about capacity). Empty today: no current filter-only column is restricted +// (`user_data.mail` is filter-only but not in `GsRestrictedColumns`). Add an entry only when a +// restricted column is intentionally allowlisted for equality lookup, with the reasoning in a +// comment next to the entry. +export const DebugFilterOnlyRestrictedExceptions: Record = {}; + /** * Invariant: `GsRestrictedColumns` is the per-role masking list that `/gs/db` applies (only * SUPER_ADMIN sees the real value). The structured `/gs/debug` endpoint does NOT apply any * such masking, so allowlisting any column also listed there would bypass the role * restriction. Throw so a future addition can't slip in silently — unless the exact - * `(table, column)` pair is registered in the exceptions map, which makes the bypass an - * explicit, documented decision instead of an accident. + * `(table, column)` pair is registered in the capacity-matching exceptions map, which makes + * the bypass an explicit, documented decision instead of an accident. + * + * Capacity is part of the approval: a restricted column in `columns` needs an entry in the + * selectable map (`DebugRestrictedOverlapExceptions`); a restricted column in + * `filterOnlyColumns` needs an entry in the filter-only map + * (`DebugFilterOnlyRestrictedExceptions`). The two maps do not share exceptions — moving a + * column between capacities fails module load until the approval is moved too, so a + * filter-only exception cannot silently upgrade into full disclosure. * - * The counter-check keeps the exceptions map honest: an exception only means anything for a - * pair that actually overlaps and is actually allowlisted. A stale entry (column dropped from - * either list) would silently keep a future re-add unguarded, so it fails loudly too. + * The counter-checks keep both exceptions maps honest: a selectable exception is stale unless + * the column is in `columns`; a filter-only exception is stale unless the column is in + * `filterOnlyColumns`; an entry present in both maps for the same pair is an error (approval + * must be unambiguous); and every entry must still be in `GsRestrictedColumns`. * - * Kept as a pure function over its three inputs so the guard itself is testable with synthetic + * Kept as a pure function over its four inputs so the guard itself is testable with synthetic * fixtures — asserting the real constants only proves today's data is consistent, not that the - * exception matches per column rather than per table. + * exception matches per column and capacity rather than per table. */ export function assertDebugAllowlistInvariants( restrictedColumns: Record, allowedColumns: Record, - exceptions: Record, + selectableExceptions: Record, + filterOnlyExceptions: Record, ): void { for (const [table, restricted] of Object.entries(restrictedColumns)) { if (!Object.hasOwn(allowedColumns, table)) continue; const spec = allowedColumns[table]; - const excepted = Object.hasOwn(exceptions, table) ? exceptions[table] : []; + const selectableExcepted = Object.hasOwn(selectableExceptions, table) ? selectableExceptions[table] : []; + const filterOnlyExcepted = Object.hasOwn(filterOnlyExceptions, table) ? filterOnlyExceptions[table] : []; for (const col of restricted) { - if (spec.columns.includes(col) && !excepted.includes(col)) + if (spec.columns.includes(col) && !selectableExcepted.includes(col)) throw new Error( `DebugAllowedColumns['${table}'] contains '${col}' which is in GsRestrictedColumns; ` + `the /gs/debug endpoint does not apply role masking. Remove it from DebugAllowedColumns ` + `or register it in DebugRestrictedOverlapExceptions with a documented reason.`, ); + // Same restriction for filter-only columns: a restricted column must not become a + // WHERE key unless the pair is an explicit, documented filter-only exception. + if (spec.filterOnlyColumns?.includes(col) && !filterOnlyExcepted.includes(col)) + throw new Error( + `DebugAllowedColumns['${table}'].filterOnlyColumns contains '${col}' which is in GsRestrictedColumns; ` + + `the /gs/debug endpoint does not apply role masking. Remove it from filterOnlyColumns ` + + `or register it in DebugFilterOnlyRestrictedExceptions with a documented reason.`, + ); } } - for (const [table, excepted] of Object.entries(exceptions)) { + for (const [table, excepted] of Object.entries(selectableExceptions)) { for (const col of excepted) { if (!(Object.hasOwn(restrictedColumns, table) && restrictedColumns[table].includes(col))) throw new Error( `DebugRestrictedOverlapExceptions['${table}'] lists '${col}', which is not in ` + `GsRestrictedColumns['${table}']; the exception is stale — remove it.`, ); + // Selectable exceptions approve `columns` access only — presence in filterOnlyColumns + // does not keep this entry live. if (!(Object.hasOwn(allowedColumns, table) && allowedColumns[table].columns.includes(col))) throw new Error( `DebugRestrictedOverlapExceptions['${table}'] lists '${col}', which is not in ` + - `DebugAllowedColumns['${table}']; the exception is stale — remove it.`, + `DebugAllowedColumns['${table}'].columns; the exception is stale — remove it.`, + ); + if (Object.hasOwn(filterOnlyExceptions, table) && filterOnlyExceptions[table].includes(col)) + throw new Error( + `('${table}', '${col}') is registered in both DebugRestrictedOverlapExceptions and ` + + `DebugFilterOnlyRestrictedExceptions; approval must be unambiguous about capacity — ` + + `remove one of the entries.`, + ); + } + } + + for (const [table, excepted] of Object.entries(filterOnlyExceptions)) { + for (const col of excepted) { + if (!(Object.hasOwn(restrictedColumns, table) && restrictedColumns[table].includes(col))) + throw new Error( + `DebugFilterOnlyRestrictedExceptions['${table}'] lists '${col}', which is not in ` + + `GsRestrictedColumns['${table}']; the exception is stale — remove it.`, + ); + // Filter-only exceptions approve `filterOnlyColumns` access only. + if ( + !( + Object.hasOwn(allowedColumns, table) && + Object.hasOwn(allowedColumns[table], 'filterOnlyColumns') && + allowedColumns[table].filterOnlyColumns && + allowedColumns[table].filterOnlyColumns.includes(col) + ) + ) + throw new Error( + `DebugFilterOnlyRestrictedExceptions['${table}'] lists '${col}', which is not in ` + + `DebugAllowedColumns['${table}'].filterOnlyColumns; the exception is stale — remove it.`, + ); + if (Object.hasOwn(selectableExceptions, table) && selectableExceptions[table].includes(col)) + throw new Error( + `('${table}', '${col}') is registered in both DebugRestrictedOverlapExceptions and ` + + `DebugFilterOnlyRestrictedExceptions; approval must be unambiguous about capacity — ` + + `remove one of the entries.`, + ); + } + } + + // filterOnlyColumns must stay strictly narrower than `columns` / `jsonbColumns` and free of + // duplicates. Overlap with `columns` would silently make the value selectable; overlap with + // `jsonbColumns` would open path access; duplicates are almost certainly a config mistake. + for (const [table, spec] of Object.entries(allowedColumns)) { + if (!Object.hasOwn(spec, 'filterOnlyColumns') || !spec.filterOnlyColumns) continue; + const seen = new Set(); + for (const col of spec.filterOnlyColumns) { + if (seen.has(col)) + throw new Error(`DebugAllowedColumns['${table}'].filterOnlyColumns has duplicate entry '${col}'`); + seen.add(col); + if (spec.columns.includes(col)) + throw new Error( + `DebugAllowedColumns['${table}'].filterOnlyColumns contains '${col}' which is also in columns; ` + + `a filter-only column must never be selectable.`, + ); + if (spec.jsonbColumns?.includes(col)) + throw new Error( + `DebugAllowedColumns['${table}'].filterOnlyColumns contains '${col}' which is also in jsonbColumns; ` + + `a filter-only column must never support jsonb path access.`, ); } } } -assertDebugAllowlistInvariants(GsRestrictedColumns, DebugAllowedColumns, DebugRestrictedOverlapExceptions); +assertDebugAllowlistInvariants( + GsRestrictedColumns, + DebugAllowedColumns, + DebugRestrictedOverlapExceptions, + DebugFilterOnlyRestrictedExceptions, +); // Support endpoint export enum SupportTable { diff --git a/src/subdomains/generic/gs/gs.controller.ts b/src/subdomains/generic/gs/gs.controller.ts index 4bc1bcdf2f..2c81da6717 100644 --- a/src/subdomains/generic/gs/gs.controller.ts +++ b/src/subdomains/generic/gs/gs.controller.ts @@ -53,8 +53,9 @@ export class GsController { } // Structured debug endpoint. Takes a JSON description of the query (table, select, where, - // group/order/limit) and emits SQL via QueryBuilder with parameter binding — no raw SQL is - // accepted, parsed, or interpolated. + // group/order/limit) and emits hand-built SQL with bound parameters via `dataSource.query` — + // no raw SQL is accepted, parsed, or interpolated. + @Post('debug') @ApiBearerAuth() @ApiExcludeEndpoint() diff --git a/src/subdomains/generic/gs/gs.service.ts b/src/subdomains/generic/gs/gs.service.ts index 83f23c0a87..d5bf66acd6 100644 --- a/src/subdomains/generic/gs/gs.service.ts +++ b/src/subdomains/generic/gs/gs.service.ts @@ -42,6 +42,7 @@ import { } from './dto/debug-query.dto'; import { DebugAllowedColumns, + DebugFilterOnlyAllowedOps, DebugMaxResults, DebugQueryAuditPrefix, DebugTableSpec, @@ -52,8 +53,9 @@ import { import { SupportDataQuery, SupportReturnData } from './dto/support-data.dto'; // Mutable state carried through the /gs/debug SQL emitters. Holds the bound parameter -// array, the alias set (for ORDER/GROUP BY resolution), and a predicate counter for the -// WHERE-tree depth/size caps. Constructed once per query in executeDebugQuery. +// array, the alias set (for ORDER/GROUP BY resolution), a predicate counter for the +// WHERE-tree depth/size caps, and a filter-only leaf counter (at most one per query). +// Constructed once per query in executeDebugQuery. interface DebugQueryEmitCtx { table: string; spec: DebugTableSpec; @@ -65,6 +67,10 @@ interface DebugQueryEmitCtx { // declaring an alias whose text matches a physical-but-not-allowlisted column. aliases: Map; predicateCount: number; + // Number of filter-only WHERE leaves seen so far in this query. At most one is allowed + // anywhere in the tree (including nested and/or branches) so OR/AND of several `=` leaves + // cannot re-enable multi-candidate batching after `IN` was removed for filter-only columns. + filterOnlyCount: number; } @Injectable() @@ -235,6 +241,13 @@ export class GsService { // explicit code changes. // - LIMIT is a numeric DTO field, clamped at DebugMaxResults. No string substring scan. async executeDebugQuery(dto: DebugQueryDto, userIdentifier: string): Promise { + // Audit log emitted FIRST — before the table allowlist check or any emit/validate step + // can throw — so every accepted-shape request is attributable for forensics, including + // unknown-table probes. WHERE leaf values may carry PII (LIKE patterns over mail / IBAN); + // redact them. Bound parameters already protect the SQL string; we shouldn't undo that + // via the verbose log. + this.logger.verbose(`${DebugQueryAuditPrefix}${userIdentifier}: ${this.serializeDebugQueryForAudit(dto)}`); + // `Object.hasOwn` so prototype keys like `__proto__` / `constructor` / `toString` don't // pass the `if (!spec)` guard (they'd otherwise return `Object.prototype` and crash later // with a 500). Use the allowlist as a real lookup, not a `in`/index probe. @@ -243,18 +256,13 @@ export class GsService { } const spec = DebugAllowedColumns[dto.table]; - // Audit log emitted FIRST — before any emit/validate step can throw — so a malformed or - // probing request (bad column, oversized IN list, etc.) is still recorded for forensics. - // WHERE leaf values may carry PII (LIKE patterns over mail / IBAN); redact them. Bound - // parameters already protect the SQL string; we shouldn't undo that via the verbose log. - this.logger.verbose(`${DebugQueryAuditPrefix}${userIdentifier}: ${this.serializeDebugQueryForAudit(dto)}`); - const ctx: DebugQueryEmitCtx = { table: dto.table, spec, params: [], aliases: new Map(), predicateCount: 0, + filterOnlyCount: 0, }; // SELECT — emit fragments in the order they appear in the DTO. Aliases are collected @@ -292,11 +300,34 @@ export class GsService { const keys = dto.select.map((item) => item.as ?? this.defaultDebugSelectAlias(item)); return { keys, rows: rows.map((r) => keys.map((k) => r[k])) }; } catch (e) { - this.logger.info(`${DebugQueryAuditPrefix}${userIdentifier} failed: ${e.message}`); + // Never log e.message or the bound parameter array. Postgres echoes offending + // parameter values in some messages (e.g. `invalid input syntax for type integer: + // "user@example.com"`), which would defeat the WHERE-value redaction on the audit + // line. SQLSTATE (`code`) identifies the failure class; severity/routine are + // value-free driver fields when present. Missing code → stable placeholder, not + // a fallback to the message. + this.logger.info( + `${DebugQueryAuditPrefix}${userIdentifier} failed: ${this.formatDebugQueryFailureDiagnostics(e)}`, + ); throw new BadRequestException('Query execution failed'); } } + // Value-free failure diagnostics for the /gs/debug catch path. Only fields that cannot + // carry bound parameter values are included (SQLSTATE / severity / routine). + private formatDebugQueryFailureDiagnostics(e: unknown): string { + const err = e as { code?: unknown; severity?: unknown; routine?: unknown } | null | undefined; + const code = typeof err?.code === 'string' && err.code.length > 0 ? err.code : ''; + const parts = [`code=${code}`]; + if (typeof err?.severity === 'string' && err.severity.length > 0) { + parts.push(`severity=${err.severity}`); + } + if (typeof err?.routine === 'string' && err.routine.length > 0) { + parts.push(`routine=${err.routine}`); + } + return parts.join(' '); + } + // --- Emitters for /gs/debug --- // Replaces every `value` field (WHERE leaf scalars or IN-list arrays) with a redaction @@ -328,14 +359,40 @@ export class GsService { return redacted.length > 500 ? `${redacted.substring(0, 500)}...` : redacted; } - // Asserts a column name is in the table allowlist; throws otherwise. Used everywhere a - // user-supplied identifier could reach SQL. - private assertDebugColumnAllowed(column: string, spec: DebugTableSpec): void { + // Asserts a column name is in the SELECT allowlist (`spec.columns` only). Filter-only + // columns are deliberately excluded so they can never appear in SELECT, aggregates, or + // jsonb path access (all three select kinds route through this helper). + private assertDebugSelectColumnAllowed(column: string, spec: DebugTableSpec): void { if (!spec.columns.includes(column)) { throw new BadRequestException(`Column '${column}' is not allowed on this table`); } } + // Asserts a column may be used as a WHERE-leaf filter. Accepts `spec.columns` plus any + // `filterOnlyColumns`. Filter-only columns are further restricted to equality-style ops + // (`DebugFilterOnlyAllowedOps`) so range/pattern operators cannot oracle-reconstruct the value. + // A filter-only leaf under any `not` ancestor is also rejected (`negated`): `NOT (mail = x)` + // is equivalent to `mail != x`, which would bypass the operator gate. Any negation above a + // filter-only leaf is refused — double negation is NOT cancelled out. + private assertDebugFilterColumnAllowed( + column: string, + op: DebugWhereOp, + spec: DebugTableSpec, + negated: boolean, + ): void { + if (spec.columns.includes(column)) return; + if (spec.filterOnlyColumns?.includes(column)) { + if (negated) { + throw new BadRequestException(`Filter-only column '${column}' cannot be used inside a NOT`); + } + if (!DebugFilterOnlyAllowedOps.includes(op)) { + throw new BadRequestException(`Operator '${op}' is not allowed on filter-only column '${column}'`); + } + return; + } + throw new BadRequestException(`Column '${column}' is not allowed on this table`); + } + // Validates a jsonb path string: dot-separated, each segment matches the identifier regex, // max 8 segments to bound the emitted expression. Returns the segment array. private parseDebugJsonbPath(path: string): string[] { @@ -371,7 +428,7 @@ export class GsService { // inputs. Otherwise a malformed `{kind: 'jsonb'}` (no jsonbPath) would TypeError when // synthesizing the alias and surface as 500 instead of a clean 400. private emitDebugSelectItem(item: DebugSelectItem, ctx: DebugQueryEmitCtx, position: number): string { - this.assertDebugColumnAllowed(item.column, ctx.spec); + this.assertDebugSelectColumnAllowed(item.column, ctx.spec); // Defense in depth: an explicit `as` is interpolated as `AS "${alias}"`. The DTO regex // already enforces this shape — re-check here so a future change that bypasses the DTO @@ -426,27 +483,33 @@ export class GsService { // Recursive WHERE emitter. Caps depth and predicate count to prevent JSON-tree DoS. Returns // a parenthesized SQL fragment with parameter placeholders. - private emitDebugWhere(node: DebugWhereNode, ctx: DebugQueryEmitCtx, depth: number): string { + // + // `negated` is true under any `not` ancestor. Filter-only columns are rejected in a + // negated context (see `assertDebugFilterColumnAllowed`). Entering a `not` node sets the + // flag for the whole subtree — double negation is deliberately NOT cancelled out. + private emitDebugWhere(node: DebugWhereNode, ctx: DebugQueryEmitCtx, depth: number, negated = false): string { if (depth > DebugQueryMaxWhereDepth) { throw new BadRequestException(`WHERE tree exceeds max depth of ${DebugQueryMaxWhereDepth}`); } switch (node.kind) { case 'leaf': - return this.emitDebugWhereLeaf(node, ctx); + return this.emitDebugWhereLeaf(node, ctx, negated); case 'and': case 'or': { if (!node.children?.length) { throw new BadRequestException(`'${node.kind}' node requires at least one child`); } - const parts = node.children.map((c) => this.emitDebugWhere(c, ctx, depth + 1)); + const parts = node.children.map((c) => this.emitDebugWhere(c, ctx, depth + 1, negated)); return `(${parts.join(node.kind === 'and' ? ' AND ' : ' OR ')})`; } case 'not': { if (!node.child) throw new BadRequestException("'not' node requires `child`"); - return `(NOT ${this.emitDebugWhere(node.child, ctx, depth + 1)})`; + // Set (not flip) negated for the subtree — any filter-only leaf below a NOT is refused, + // including under double negation. Ordinary allowlisted columns keep working inside NOT. + return `(NOT ${this.emitDebugWhere(node.child, ctx, depth + 1, true)})`; } default: @@ -456,7 +519,7 @@ export class GsService { // Emits one leaf predicate. Validates column-in-allowlist and op-against-value-shape, then // binds the value(s) as parameters. - private emitDebugWhereLeaf(node: DebugWhereNode, ctx: DebugQueryEmitCtx): string { + private emitDebugWhereLeaf(node: DebugWhereNode, ctx: DebugQueryEmitCtx, negated: boolean): string { if (++ctx.predicateCount > DebugQueryMaxPredicates) { throw new BadRequestException(`WHERE tree exceeds max predicates of ${DebugQueryMaxPredicates}`); } @@ -469,9 +532,18 @@ export class GsService { if (!Object.values(DebugWhereOp).includes(node.op)) { throw new BadRequestException(`Operator '${node.op}' is not allowed`); } - this.assertDebugColumnAllowed(node.column, ctx.spec); + this.assertDebugFilterColumnAllowed(node.column, node.op, ctx.spec, negated); const colSql = `"${ctx.table}"."${node.column}"`; + const isFilterOnly = !!ctx.spec.filterOnlyColumns?.includes(node.column); + // At most one filter-only predicate per query, anywhere in the WHERE tree (including + // nested and/or). Without this, OR/AND of several `=` leaves would batch candidates as + // effectively as the `IN` form that was deliberately disallowed for filter-only columns. + if (isFilterOnly) { + if (++ctx.filterOnlyCount > 1) { + throw new BadRequestException('Only one filter-only predicate is allowed per query'); + } + } switch (node.op) { case DebugWhereOp.IS_NULL: @@ -500,6 +572,19 @@ export class GsService { throw new BadRequestException(`op '${node.op}' requires a single scalar value`); } this.assertDebugScalarValue(node.value); + // Filter-only equality is case-insensitive so the debug lookup answers the same + // question the application asks (`getUsersByMail` resolves via `LOWER(mail)`) and so + // a support caller may type an address in a different case than stored. Equality + // stays equality — only letter case is forgiven; no additional information is granted. + // For `user_data.mail` the emission is index-supported by the functional index on + // `LOWER(mail)` (non-unique by design while case-collision duplicates remain pending + // merge). That index is specific to `user_data.mail`, not a general guarantee for + // every future filter-only column. Value stays bound. + // Precondition: filter-only columns must be text — `LOWER()` is unconditional here; + // a non-text column would fail at query time. Documented on `filterOnlyColumns`. + if (isFilterOnly && node.op === DebugWhereOp.EQ) { + return `LOWER(${colSql}) = LOWER($${this.bindDebugParam(node.value, ctx)})`; + } return `${colSql} ${node.op} $${this.bindDebugParam(node.value, ctx)}`; } } @@ -530,6 +615,8 @@ export class GsService { // "ip"` would bind to a physical `ip` column (if one exists on the table but isn't in the // allowlist), bypassing the allowlist for grouping. Ordinals sidestep the ambiguity. private emitDebugGroupIdent(name: string, ctx: DebugQueryEmitCtx): string { + // Filter-only columns are deliberately excluded — validate against `columns` only so + // they stay ungroupable (and never surface as a grouping key). if (ctx.spec.columns.includes(name)) return `"${ctx.table}"."${name}"`; const position = ctx.aliases.get(name); if (position !== undefined) return String(position); @@ -541,6 +628,8 @@ export class GsService { // quoted alias is safe. Kept separate from `emitDebugGroupIdent` so the GROUP BY ordinal // emission is explicit at the call site. private emitDebugOrderIdent(name: string, ctx: DebugQueryEmitCtx): string { + // Filter-only columns are deliberately excluded — validate against `columns` only so + // they stay unorderable (and never surface as a sort key). if (ctx.spec.columns.includes(name)) return `"${ctx.table}"."${name}"`; if (ctx.aliases.has(name)) return `"${name}"`; throw new BadRequestException(`'${name}' is neither an allowed column nor a select alias`);