fix: data leaks, alert-channel bypass, and two silent wrong answers - #4
Merged
Merged
Conversation
added 4 commits
September 17, 2026 18:39
RedactForTransport was a no-op on every message the agent can actually produce. The pattern was anchored with (?m)^...$, but tail() joins command output with " / " before the message is built, so there are no line starts to match and DETAIL/CONTEXT segments passed through untouched. What that leaked: a failed database load is the exact case this product exists to detect, and Postgres puts the offending value in DETAIL and the whole failing row in CONTEXT. Those reached the control plane, were stored in test_runs.results, and were lifted into alert bodies sent to Telegram, Discord, ntfy and email. Segment the message on either joiner and match per segment instead. Also covers psql's "LINE n:" echo, which quotes the failing statement text and was not in the pattern at all. RunResult.Error gets the same treatment. It carried only the credential scrub, so a restore failure shipped an unbounded list of paths out of the user's snapshot; the server accepts 4000 characters where check messages are capped at 500. The old unit test passed throughout because it fed a message containing real newlines, a shape nothing generates. Tests now use captured psql and mysql output, and a new test in the recipes package runs tail() output through RedactForTransport so the two cannot drift apart again: the bug lived in the seam between them, not in either function.
alert_channels carried a table-level insert/update grant to `authenticated`
and RLS policies that only check user_id, so any signed-in user could POST
straight to PostgREST with {"verified": true} for an address they do not own.
Verified channels receive every alert, and both halves of an alert are
submitter controlled: the title carries repo_label and the body carries the
check message. With SES out of sandbox at 50k/day and no cap on agents per
account, that is a working path to sending attacker-worded mail from our own
domain.
`verified` asserts that a message actually arrived, which is decided in Node
by sendToChannel returning. No policy or user-callable function can enforce
that, since either would be equally spoofable, so the privilege is revoked
from `authenticated` and the server flips the flag with the service role
after a real send.
Creating a channel stays a client insert, so the RLS policy and the
plan-limit gate still govern it; `verified` simply keeps its default. Editing
config stays a client update. Telegram connect now inserts unverified and
flips separately rather than inserting pre-verified, which keeps channel
creation under the same gate as every other type.
Verified against the local stack: insert and update of `verified` both 403 as
an authenticated user, normal create returns 201 with verified=false, config
edits still work, and the service role can still flip it. alerts.sh was
performing this exact bypass in its own setup and now uses the service key,
mirroring the app. All four web e2e suites pass.
resolve() checked containment lexically and then called os.Stat, which
follows symlinks. restic restores symlinks as symlinks, so the common homelab
shape — /srv/app backed up while `data` links to /var/lib/app — restored a
link pointing at the live host copy, and every check read the running system
instead of the backup.
That is a silent false PASS on the one question this tool exists to answer.
Demonstrated against the shipped binary with a real restic repository and a
healthy host database behind the link:
old: PASS, exit 0 ✓ "data/app.db" passes integrity_check
new: FAIL, exit 1 ✗ database "data/app.db" is a symlink leading outside
the restored snapshot, so its target was never backed up
Resolve both sides before comparing. Comparing a resolved path against an
unresolved root would reject everything on macOS, where the sandbox sits under
/var/folders and /var is itself a link to /private/var.
An escape is reported as its own condition rather than as "not found", because
it is a real finding: the symlink was backed up and its target was not, which
is a backup gap worth naming. resolve() returns an error instead of a bool to
carry that, and the five call sites now interpolate it.
Symlinks pointing within the snapshot still resolve normally.
…oard The agent sent the full repo string while the control plane caps repo_label at 200. Past that the whole submission was rejected, so the repo row was never created, the run never appeared on the dashboard, and stale detection could never fire for that repo — it needs an existing row. Meanwhile the run printed PASS and exited 0, and the only backstop was a 24h agent-silent alert blaming the wrong thing. Long B2 and S3 URLs and deep local paths cross 200 characters easily, and the failure is silent in the direction that matters: the user believes a repo is covered when the control plane has never heard of it. Truncate in the middle, keeping the scheme and host at the head and the path that distinguishes two repos on the same host at the tail. The label is display only — repos are identified by fingerprint — so shortening costs nothing. Cuts land on rune boundaries, and bounding bytes is sufficient because the server counts UTF-16 units, which is never more than the UTF-8 byte count.
dabelle
pushed a commit
that referenced
this pull request
Sep 19, 2026
…erification fix: data leaks, alert-channel bypass, and two silent wrong answers
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
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.
Four defects found by a pre-launch verification sweep. Two were data leaving the machine, one was an abuse vector, one was a silent wrong answer. Each fix is verified by experiment rather than inspection, and each carries a test that fails against the old behaviour.
Restored database contents reached third parties
RedactForTransportwas a no-op on every message the agent can produce. The pattern was anchored with(?m)^...$, buttail()joins command output with" / "before the message is built, so there are no line starts to match. A failed database load is the exact case this product exists to detect, and Postgres puts the offending value inDETAILand the failing row inCONTEXT. Those reached the control plane, were stored intest_runs.results, and were lifted into alert bodies sent to Telegram, Discord, ntfy and email.Segment the message on either joiner and match per segment. Also covers psql's
LINE n:echo, which quotes the failing statement and was not in the pattern at all.RunResult.Errorgets the same treatment; it previously carried only the credential scrub, so a restore failure shipped an unbounded list of paths out of the snapshot.The old test passed throughout because it fed a message containing real newlines, a shape nothing generates. A new test in the recipes package runs
tail()output throughRedactForTransport, so the two cannot drift apart again. The bug lived in the seam between two individually reasonable functions.alert_channels.verified was client-writable
A table-level grant to
authenticatedplus policies that only checkuser_idmeant any signed-in user could POST to PostgREST with{"verified": true}for an address they do not own. Verified channels receive every alert, and both halves are submitter controlled: the title carriesrepo_label, the body carries the check message. With SES out of sandbox and no cap on agents per account, that is a working path to sending attacker-worded mail from our own domain.verifiedasserts a message actually arrived, which only Node knows. No policy or user-callable function can enforce that, so the privilege is revoked and the server flips the flag with the service role after a real send. Creating a channel stays a client insert, so the RLS policy and plan-limit gate still govern it.Verified locally: insert and update of
verifiedboth 403 as an authenticated user, normal create returns 201 withverified=false, config edits still work, service role can still flip it.alerts.shwas performing this exact bypass in its own setup and now uses the service key.Checks verified data from outside the backup
resolve()checked containment lexically then calledos.Stat, which follows symlinks. restic restores symlinks as symlinks, so/srv/appbacked up whiledatalinks to/var/lib/apprestored a link to the live host copy, and checks read the running system.Demonstrated against the shipped binary with a real restic repository and a healthy host database behind the link:
Both sides are resolved before comparing, because the sandbox sits under a symlinked prefix on macOS and a one-sided check would reject every legitimate path. An escape is reported as its own condition rather than "not found", since the symlink was backed up and its target was not, which is a backup gap worth naming.
Long repo paths never reached the dashboard
The agent sent the full repo string while the server caps
repo_labelat 200. Past that the whole submission was rejected, so the repo row was never created, the run never appeared, and stale detection could never fire for that repo. The run still printed PASS and exited 0. Long B2 and S3 URLs cross 200 characters easily.Truncates in the middle, keeping scheme and host at the head and the distinguishing path at the tail, on rune boundaries.
Verification
Full Go suite,
-race, and golangci-lint clean. All three agent e2e suites and all four web e2e suites pass. Thealert_channelschange was exercised against a local Supabase stack with both attack vectors and all legitimate flows.Not addressed here
The redaction fix is a denylist. It covers every vector found, but a future database output format that echoes row data leaks by default. The stricter version reduces load failures to an error class plus SQLSTATE and never forwards database text, which changes what users see in alerts and is worth deciding separately.