Skip to content

METADATA records FullLine but never before/after: pkg/scan's context contract omits it, and #286's per-type hash fix would exclude it #290

Description

@sectoramen

Found while reviewing #285. Adjacent to #286 but not covered by it, and #286's preferred fix would leave it out by construction.

The gap

#285 documents the validators that record no context, and #286 tracks closing it. Both treat the world as two shapes — records all three fields, or records nothing. There is a third: the metadata validator populates Context.FullLine and never BeforeText/AfterText.

internal/validators/metadata/metadata_validator.go builds detector.ContextInfo{FullLine: line} at ~26 sites and attaches it as-is, e.g. :799 and :816. Measured through the new public fields on a real .docx (a docProps/core.xml with dc:creator):

metadata/AUTHOR_INFO       before="" after="" full="Author: Michael Thompson"
metadata/LAST_MODIFIED_BY  before="" after="" full="LastModifiedBy: Michael Thompson"

So ContextBefore/ContextAfter are always empty for METADATA findings, however much text surrounds the value.

Why it matters for #285's contract

The Finding doc comment enumerates the exception:

As of this release SECRETS, PERSON_NAME and CLOUD_RESOURCES never populate them

A caller branching on that list concludes METADATA findings carry before/after. They never do. TestValidatorsWithoutContextAreDocumented locks only the three named validators, so the fourth shape is unguarded as well as undocumented.

This gets worse rather than better when #286 lands: the natural doc edit once those three are backfilled is "all validators populate context", which would be actively false for METADATA. Worth correcting in the contract before #286 rewrites it.

Why it matters for #286's fix

#286's option (2) — "compute the context-free variant only for the three affected types" — excludes METADATA.

But metadata findings sit on the same cliff. hashSensitiveData returns "" for empty input (internal/suppressions/suppression.go:305-311), so today a metadata finding's context_hash component is the empty string. Attach BeforeText/AfterText to metadata at any point in the future and that component becomes a real hash, the composite changes, and every existing operator rule for a metadata finding stops matching — the exact failure #286 exists to prevent, one type over, with no candidate covering it.

FullLine is already in metadata's hash, so it is only the before/after half that is exposed.

If option (1) (a context-free candidate for all types) is chosen, this is moot. If option (2) is chosen, the per-type list should be data-driven — "types whose context shape may change" — rather than the three names hard-coded, or METADATA should be included in it from the start.

Suggested change

Independent of #286:

  1. Name METADATA in the Finding doc comment as a distinct case: FullLine populated, before/after never. (Several metadata paths also set Text: line, so FullLine == Text there — worth a word, since it means the new field is not additional exposure for those findings.)
  2. Extend TestValidatorsWithoutContextAreDocumented with a per-field expectation instead of an all-three-empty assertion, so the FullLine-only shape is locked too.

Happy to send that as a small follow-up to #285 if the maintainers prefer it separate from the #286 work.

How it was found

Probed all 19 validators through pkg/scan.ScanText/ScanFile and cross-checked against every ContextInfo literal in internal/validators. Full matrix: 15 validators populate all three; secrets, personname and cloudresources populate none (#286); metadata populates FullLine only (this issue); SOCIAL_MEDIA_CLUSTER borrows another match's context (filed separately).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions