redactMetadata in internal/search/search.go redacts ten sessions.Metadata fields and leaves four caller-provided string fields untouched: Tag, SourceModelID, SpecUserComment and SpecRejectReason. RedactResult embeds that metadata in every Hit, and zero search --json prints the result directly, so text in any of those four reaches stdout unredacted.
Found while reviewing #878, where CodeRabbit raised it. It is not that PR's regression and its import path is already defended, so filing it separately.
What is and is not exposed today:
SourceModelID is set by the session importer through agentsessions.DisplayField, which redacts and strips controls before the value is stored. Driven with a sk-ant-api03- key in a foreign model id, both contiguous and tab-split, and with an escape sequence: all came back redacted or stripped. So the field is covered by accident of where it is written, not by redactMetadata.Tag on an imported session is sessions.ImportedSessionTag(agent, sourceID), built from the adapter name and the foreign session id rather than free text.SpecUserComment and SpecRejectReason carry spec-mode review text straight from the user, and nothing sanitizes them on the way in or out.
So the live exposure is the two spec fields, and the other two are one refactor away from becoming exposed, since nothing at the search boundary states the rule.
The comment on WorkspaceKey a few lines up already makes the right argument, that a presentation copy should not carry operational identity, and then the function keeps hand-listing fields. A struct gains a field and the redaction silently does not cover it, which is how these four got here.
Suggested fix, either:
- Return an explicit presentation type from search with only the fields the CLI renders, so a new
Metadata field is absent by default rather than exposed by default, or
- Keep
Metadata but redact every string field by construction rather than by name, and add a test that fails when a new string field is added without coverage.
Option 1 matches the WorkspaceKey reasoning. Either way, a regression on one of the spec fields.
redactMetadataininternal/search/search.goredacts tensessions.Metadatafields and leaves four caller-provided string fields untouched:Tag,SourceModelID,SpecUserCommentandSpecRejectReason.RedactResultembeds that metadata in everyHit, andzero search --jsonprints the result directly, so text in any of those four reaches stdout unredacted.Found while reviewing #878, where CodeRabbit raised it. It is not that PR's regression and its import path is already defended, so filing it separately.
What is and is not exposed today:
SourceModelIDis set by the session importer throughagentsessions.DisplayField, which redacts and strips controls before the value is stored. Driven with ask-ant-api03-key in a foreign model id, both contiguous and tab-split, and with an escape sequence: all came back redacted or stripped. So the field is covered by accident of where it is written, not byredactMetadata.Tagon an imported session issessions.ImportedSessionTag(agent, sourceID), built from the adapter name and the foreign session id rather than free text.SpecUserCommentandSpecRejectReasoncarry spec-mode review text straight from the user, and nothing sanitizes them on the way in or out.So the live exposure is the two spec fields, and the other two are one refactor away from becoming exposed, since nothing at the search boundary states the rule.
The comment on
WorkspaceKeya few lines up already makes the right argument, that a presentation copy should not carry operational identity, and then the function keeps hand-listing fields. A struct gains a field and the redaction silently does not cover it, which is how these four got here.Suggested fix, either:
Metadatafield is absent by default rather than exposed by default, orMetadatabut redact every string field by construction rather than by name, and add a test that fails when a new string field is added without coverage.Option 1 matches the
WorkspaceKeyreasoning. Either way, a regression on one of the spec fields.