From 540985d515c7e9ee9e16be7cd66b8651fd7cb2a9 Mon Sep 17 00:00:00 2001 From: thetechnologist1911 Date: Sat, 19 Sep 2026 09:35:46 -0400 Subject: [PATCH] fix(agent): redact row values MySQL inlines into its error line A verification pass caught a leak the earlier redaction fix missed. Postgres isolates row data in DETAIL and CONTEXT, so dropping whole segments removes it. MySQL does not: it puts the offending value on the same ERROR line as the reason, so segment filtering never touched it and a user's data went out on the wire. before: Duplicate entry 'alice@example.com' for key 'users.email' after: Duplicate entry '[redacted]' for key 'users.email' Only the quoted value is replaced. Dropping the whole line would take the reason with it, and the identifier after "for key" or "for column" is schema rather than data, so naming the constraint that failed still works. The test that should have caught this carried dave@example.com in its input and only asserted the DETAIL line was hidden, so the suite stayed green while the address shipped. That is the same shape as the original bug: a test asserting something adjacent to the thing that matters. It now asserts the value is gone and the constraint name survives. SECURITY.md stops overclaiming. This is pattern matching against known engine output, it has now been wrong twice, and the page says so and points at standalone mode as the version of the promise that does not depend on us keeping up with database vendors. Also renames the cron example's results file: --json emits pretty-printed objects, so a .jsonl extension promised one-per-line and would break the obvious `tail -1 | jq`. --- SECURITY.md | 14 +++++++++++--- .../recipes/transport_redaction_test.go | 8 ++++++-- agent/internal/report/redact_test.go | 12 ++++++++++++ agent/internal/report/report.go | 18 +++++++++++++++++- docs/quickstart.md | 6 +++--- 5 files changed, 49 insertions(+), 9 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 59ac129..2c355ba 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -56,9 +56,17 @@ standalone and skip the cloud entirely. Strings go through scrubbing and redaction before they're logged or transmitted, including errors from cleanup failures. Credentials embedded in -repository URLs are stripped before anything can reach a report, and output -from database checks is stripped of the segments that echo row values, since a -failed load quotes the data that failed. +repository URLs are stripped before anything can reach a report. + +Output from database checks gets the same treatment, because a failed load +quotes the data that failed: Postgres isolates row values in DETAIL and +CONTEXT, MySQL inlines them in the error itself, and both are removed. Be +aware this part is pattern matching against known shapes rather than a +guarantee. It has been wrong twice, so assume a database engine can invent a +new way to quote your data that we have not covered yet. If a database failure +must never carry row values off the machine under any circumstances, run the +agent standalone and skip the cloud. That is the only version of this promise +that does not depend on us keeping up. The agent polls outbound over HTTPS and opens no inbound ports. diff --git a/agent/internal/recipes/transport_redaction_test.go b/agent/internal/recipes/transport_redaction_test.go index 0657af9..8f8d122 100644 --- a/agent/internal/recipes/transport_redaction_test.go +++ b/agent/internal/recipes/transport_redaction_test.go @@ -36,10 +36,14 @@ func TestTailOutputSurvivesTransportRedaction(t *testing.T) { mustHide: []string{"a@example.com", "b@example.com"}, }, { - name: "mysql detail line", + // MySQL puts the row value on the ERROR line itself rather than in + // a DETAIL segment. An earlier version of this case carried + // dave@example.com in the input and never asserted it was hidden, + // so the suite stayed green while the address went out on the wire. + name: "mysql inlines the row value on the ERROR line", raw: "ERROR 1062 (23000) at line 3: Duplicate entry 'dave@example.com' for key 'users.email'\n" + "DETAIL: row 3 rejected", - mustHide: []string{"row 3 rejected"}, + mustHide: []string{"dave@example.com", "row 3 rejected"}, }, } diff --git a/agent/internal/report/redact_test.go b/agent/internal/report/redact_test.go index 4f2cf89..0d5b7ac 100644 --- a/agent/internal/report/redact_test.go +++ b/agent/internal/report/redact_test.go @@ -64,6 +64,18 @@ func TestRedactForTransportStripsDBDetail(t *testing.T) { mustHide: []string{"a@example.com", "b@example.com", "LINE 1"}, mustKeep: "dump failed", }, + { + name: "mysql duplicate entry, value inline on the ERROR line", + msg: `dump "db/my.sql" failed to load into mysql: ERROR 1062 (23000) at line 3: Duplicate entry 'alice@example.com' for key 'users.email'`, + mustHide: []string{"alice@example.com"}, + mustKeep: "for key 'users.email'", + }, + { + name: "mysql incorrect value for column", + msg: `load failed: ERROR 1366 (HY000) at line 2: Incorrect integer value: 'not-a-number-carol' for column 'age' at row 7`, + mustHide: []string{"not-a-number-carol"}, + mustKeep: "for column 'age'", + }, { name: "mysql HINT", msg: `load failed / HINT: row 4 value "carol@example.com" is invalid`, diff --git a/agent/internal/report/report.go b/agent/internal/report/report.go index fe8464b..e1309b7 100644 --- a/agent/internal/report/report.go +++ b/agent/internal/report/report.go @@ -81,6 +81,21 @@ var dbDetailRe = regexp.MustCompile(`(?i)^\s*(DETAIL|HINT|CONTEXT|LINE\s+\d+|Key // joined from, whichever joiner produced it. var segmentRe = regexp.MustCompile(`\n| / `) +// mysqlRowValueRe matches the places MySQL inlines a value from the user's +// data into an error. +// +// Postgres isolates row data in DETAIL and CONTEXT, so dropping whole segments +// removes it. MySQL does not: it puts the offending value on the same ERROR +// line as the reason ("Duplicate entry 'alice@example.com' for key +// 'users.email'"), so segment filtering alone let it through. Dropping the +// whole line instead would take the reason with it, which is the thing the +// message exists to convey. +// +// Only the quoted value is replaced. The identifier after "for key" or "for +// column" is schema rather than data, and naming the constraint that failed is +// most of the diagnostic worth. +var mysqlRowValueRe = regexp.MustCompile(`(?i)\b(Duplicate entry|value:)\s+'[^']*'`) + // RedactForTransport prepares a check message to leave the machine. Check // messages are the one field that can embed raw command output from the // user's restored databases, so anything sent to the control plane is stripped @@ -93,7 +108,8 @@ func RedactForTransport(msg string) string { segs[i] = "[redacted]" } } - msg = Scrub(strings.Join(segs, " / ")) + msg = mysqlRowValueRe.ReplaceAllString(strings.Join(segs, " / "), "$1 '[redacted]'") + msg = Scrub(msg) const max = 500 if len(msg) > max { msg = msg[:max] + "…" diff --git a/docs/quickstart.md b/docs/quickstart.md index b303312..dd8f1c5 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -90,11 +90,11 @@ the number that matters on the day you actually need it. ## 4. Make it automatic (2 min) Cron (uses exit codes: 0 pass, 1 fail, 2 error). Results go to stdout and -progress to stderr, so keep them apart and the results file stays one JSON -object per run: +progress to stderr, so keep them apart and the results file collects one +pretty-printed JSON object per run: ``` -0 3 * * 0 RESTIC_PASSWORD="$(cat /etc/restorable/pw)" restorable test --config /etc/restorable/agent.yaml --json >> /var/log/restorable.jsonl 2>> /var/log/restorable.log +0 3 * * 0 RESTIC_PASSWORD="$(cat /etc/restorable/pw)" restorable test --config /etc/restorable/agent.yaml --json >> /var/log/restorable-results.json 2>> /var/log/restorable.log ``` Or run the built-in daemon — add `schedule: "0 3 * * 0"` to `agent.yaml` and: