Skip to content

redact, gateway, usage: what the request archive and the OTLP bodies … - #1074

Open
TryWorld2026 wants to merge 1 commit into
yetone:mainfrom
TryWorld2026:fix/redact-scrub-keeps-user-rules
Open

TryWorld2026 wants to merge 1 commit into
yetone:mainfrom
TryWorld2026:fix/redact-scrub-keeps-user-rules

Conversation

@TryWorld2026

@TryWorld2026 TryWorld2026 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

redact, gateway, usage: what the request archive and the OTLP bodies keep loses a masking rule of the user's own (#195)

What is wrong

A rule the user added in Settings → Privacy for a key format magpie's own rules
do not know — a relay's rz_ key, say — went out of what the vendor was
sent and stayed in what magpie kept.

redact.Scrub, ScrubHeader and ScrubJSON ran with Options{Secrets: true}
and nothing else, so Options.Rules never reached them. The vendor side goes
through redact.MaskJSON(body, redactionOptions()), which carries the full
options — so the two halves of the same request disagreed about what a secret
is. Three "keep, don't send" paths held the value in plaintext:

  • the request archive's bodies both ways, its headers, its query and its error lines;
  • the bodies the gateway's OTLP export carries;
  • the bodies an agent's own session files give that export.

The user's masking words and personal data went the same way: the masking covers
them, the scrubbing dropped them.

What changed

redact now takes the options it is given: ScrubWith, ScrubHeaderWith and
ScrubJSONWith, with Scrub, ScrubHeader and ScrubJSON as thin wrappers
passing Options{Secrets: true} exactly as before — every existing caller's
behaviour is unchanged, and TestScrubWithOptions pins both the new entries
and that the old ones still take only the secrets out.

  • Settings.Redaction() puts the settings → redact.Options mapping in one
    place, so redactionOptions() and the scrubbing paths cannot drift apart.
  • scrubOptions() is the archive's and the OTLP bodies' options: what the user
    asked masked, and every secret beside it, masking on or not. What they
    keep goes to the user's own bucket or their collector, never to a vendor, so
    a secret goes from it either way — which is the archive's existing contract,
    kept.
  • The archive's upload takes every part of what it uploads out with those
    options: the error and fallback lines, the query, both sides' headers, both
    sides' bodies.
  • bodyForExport (an agent's session export) and sessionBody (the OTLP
    bodies an agent's own files give) read the same options.

One semantic boundary this PR does not touch

A user's masking rule still only takes effect while Mask secrets is on: that
is internal/settings/settings.go's documented meaning and what
internal/redact/rules_test.go and internal/gateway/redact_test.go's
rules_off case pin. Making a rule work on its own is a policy change for the
maintainer, so this PR leaves it alone and forces only the secrets on the
keep-don't-send side.

Semantic change

  • Before: redact.Scrub* took magpie's own secrets out and nothing else, so
    the archive and the OTLP bodies kept a user rule's match in plaintext.
  • After: those paths carry the user's own rules, words and personal data, and
    the secrets on top of them.
  • Reference: the Privacy settings and Redaction in
    internal/settings/settings.go; implementation ScrubWith and friends in
    internal/redact/scrub.go, scrubOptions in internal/gateway/redact.go,
    upload in internal/gateway/archive.go, bodyForExport in
    internal/gateway/capture.go, sessionBody in internal/usage/otel_sessions.go.

No reference picks this up: docs/reference.md's OTLP export section describes
what a trace carries and says "Header checks never upload history or bodies",
but neither it nor any docs/subsystems/ page states what masking a kept body
takes out — the request archive is not documented at all. So there is no
reference to correct here. If you would rather one exist for it, say so and I
will write it.

Verification

TestArchiveKeepsTheUsersRules (masking on and off),
TestBodyForExportKeepsTheUsersRules and TestSessionBodyKeepsTheUsersRules
fail without the change: the archived reply, the archived request with masking
off, the exported body and the session body each hold
rz_RelayKey1234567 in plaintext. TestScrubWithOptions holds the new entries
and that the old ones still take only the secrets out.

The tests use an rz_ prefix no built-in rule knows, so a match by an internal
rule cannot make them pass for the wrong reason; the masking on/off subtests
cover the placeholder and the direct-scrub paths separately.

  • go test -tags nogui for internal/redact, internal/usage (whole package)
    and the archive, capture and redaction tests of internal/gateway;
  • go vet -tags nogui on redact, gateway, usage and settings;
  • gofmt on LF-normalized copies of each changed file, the worktree being CRLF;
    go build -tags nogui for GOOS=windows, GOOS=darwin and GOOS=linux.
  • internal/gateway's own suite (about 13 minutes here) is unaffected but for
    one failure that is the same on the base commit:
    TestSweepBridgeProjectsTakesOnlyTheBridgesFolders, which needs a symlink
    privilege this Windows box does not grant. That was the only --- FAIL in the
    whole run, and it fails identically against the unpatched base.
  • Real use: the archive path was not exercised against a real archive bucket.
    The OTLP bodies were checked at sessionBody's level, not against a live
    collector.

TestScrubWithOptions is the one test of the four that cannot run on the base
commit by itself: it names ScrubWith and its siblings, which this PR is what
adds, so on the base the package does not build. To get a real answer for it I
ran it on a scratch copy of the base with those three names defined as plain
delegation to the base's own Scrub / ScrubHeader / ScrubJSON — the base's
semantics, options dropped, nothing faked — and it failed on the same symptom.
The other three tests need no such copy: they fail on the base as it stands.

…keep loses a masking rule of the user's own (yetone#195)

A rule the user added in Settings > Privacy for a key format magpie's own
rules don't know - a relay's rz_ key say - went out of what the vendor was
sent and stayed in what was kept. Scrub, ScrubHeader and ScrubJSON ran with
Options{Secrets: true} and nothing else, so the request archive's bodies
both ways, its headers, its query and its error lines, the bodies the
gateway's OTLP export carries and the bodies an agent's own session files
give that export all held the value in plaintext while the vendor only ever
saw a placeholder. The user's words and personal data went the same way:
the masking covers them, the scrubbing dropped them.

Scrub now takes the options it is given - ScrubWith, ScrubHeaderWith,
ScrubJSONWith - with Scrub, ScrubHeader and ScrubJSON as they were, secrets
only, and settings' mapping to redact.Options in one place
(Settings.Redaction). The archive keeps what the request was masked with on
its wire and takes every part of what it uploads out with it; the bodies the
gateway's OTLP export carries and the session bodies read the same options,
with every secret taken out whether masking is on or not, as the archive
always has - what they keep goes to the user's bucket or their collector,
never to a vendor. redacted's early return names the rules beside the words,
which changes nothing on the way to the vendor: mask runs the user's rules
with the secrets (settings' "masked with the secrets while Redact is on",
TestCountTokensRedacted/rules_off), so a rule on its own masks nothing until
that is decided otherwise.

TestArchiveKeepsTheUsersRules (masking on and off), TestBodyForExportKeeps-
TheUsersRules and TestSessionBodyKeepsTheUsersRules fail without the change:
the archived reply, the archived request with masking off, the exported body
and the session body each hold rz_RelayKey1234567 in plaintext.
TestScrubWithOptions holds the new entries and that the old ones still take
only the secrets out.

Verified on Windows: go test -tags nogui for internal/redact (11 tests),
internal/usage (whole package, 57s) and the archive, capture and redaction
tests of internal/gateway; gofmt on a LF-normalized copy of each changed
file, the worktree being CRLF (core.autocrlf); go vet -tags nogui on redact,
gateway, usage and settings; go build -tags nogui for GOOS=windows,
GOOS=darwin and GOOS=linux. internal/gateway's own suite (about 17 minutes
here) is unaffected but for two that are the same on the base commit:
TestSweepBridgeProjectsTakesOnlyTheBridgesFolders, which needs a symlink
privilege this Windows box does not grant, and TestPluginStreamStalledClient-
KeepsOtherRequestsMoving, which fails 3 runs in 10 both with and without
this change (a live magpie on this box is polled from the LAN while the
suite runs).
@TryWorld2026
TryWorld2026 force-pushed the fix/redact-scrub-keeps-user-rules branch from 2025ac8 to 89fcf53 Compare October 7, 2026 13:29

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant