Skip to content

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

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

TryWorld2026 wants to merge 4 commits 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.

@TryWorld2026
TryWorld2026 force-pushed the fix/redact-scrub-keeps-user-rules branch from 1335b44 to 2025ac8 Compare October 7, 2026 00:59
@TryWorld2026
TryWorld2026 force-pushed the fix/redact-scrub-keeps-user-rules branch 2 times, most recently from 1b57a12 to cd5436e Compare October 10, 2026 10:50
TryWorld2026 and others added 4 commits October 10, 2026 18:55
…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).
scrubOptions forced Secrets on so the archive and the OTLP bodies would
take every secret out whether or not masking was on for the vendor,
which is the archive's contract. But redact.mask gates the user's own
rules on Secrets too, so forcing it switched those on as well: a user
with Mask secrets off and a rule of their own had that rule applied to
what magpie kept, while the vendor side still ignored it. The two halves
of the same request then disagreed the other way round from the one this
branch set out to fix.

It is also against the documented meaning: settings has RedactRules as
the user's rules "masked with the secrets while Redact is on", and
redact_test.go's rules_off case pins that on the vendor side.

scrubOptions now takes the settings' Personal and Words with it and leaves
the rules behind, so what is forced on is magpie's own secrets and
nothing else. The words and personal data follow their own switches as
they did.

The three tests this branch added asserted the behaviour that is now
gone, so they are corrected to the boundary instead: with masking on the
user's rule goes, with it off the rz_ relay key is kept, since no rule
of magpie's knows that prefix. What the branch is for — a relay key of
the user's own no longer sitting in the archive in plaintext while
masking is on — is unchanged and still pinned.
sessionBody still opened Secrets on its own, as it did before the
archive's scrubbing took the user's rules into account, so the two
halves of the same request disagreed: a relay key of the user's own
went out of the request the archive kept with Mask secrets on, and
stayed in the body the OTLP export carried to their collector. The
export keeps a conversation an agent wrote to disk, and what it keeps
is sent nowhere, so it must take the same out of it.

It now takes what gateway.scrubOptions takes: every secret of
magpie's own either way, and the user's words and personal data with
them - but not their rules, which redact.mask gates on the secrets
that are forced on here, so forcing them would switch the user's rule
on for what is kept while the vendor side left it off.

The three checks this branch added asserted the behaviour the last
commit took away, so they are corrected to the boundary instead:
masking on and masking off both keep the rz_ relay key, since no rule
of magpie's knows that prefix, and both lose a secret magpie's own
rules know.

Verified here with go1.27.2 on Windows: internal/redact and
internal/usage pass whole; internal/gateway fails TestSweepBridgeProjects-
TakesOnlyTheBridgesFolders and TestCodexPoolSaysTheClientsVersion, which
fail the same way on origin/main on this box (a symlink privilege it
does not grant, and a version probe).
… its own key

main's 3c7473e (a value masked once is masked again wherever a request
has it) keeps a table of what has been masked and applies it whatever the
options say, so these checks read the wrong answer once another test, or
another case of the same test, has masked the key they use: ScrubJSON, Scrub
and ScrubHeader were asked whether they take only magpie's own secrets out
after MaskJSON had just put the relay key in that table, and the archive's
masking_off case was asked the same question after its masking_on sibling.

Each case now carries a key of its own, and the vendor echoes the key the
agent sent rather than a fixed one, so the masking_off case is answered by
what its own key did. The scrub check asks its no-options question first,
before anything has masked the key it asks about.

Verified on Windows with go1.27.2: TestScrubWithOptions, the whole
internal/redact package, TestArchiveKeepsTheUsersRules (both cases) and
TestBodyForExportKeepsTheUsersRules pass; go vet ./internal/redact and
./internal/gateway pass for windows/amd64, darwin/arm64 and linux/amd64;
gofmt is clean on LF-normalised copies.
@TryWorld2026
TryWorld2026 force-pushed the fix/redact-scrub-keeps-user-rules branch from cd5436e to ca7c9ed Compare October 10, 2026 11:02

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