detect: header detection can over-match and delete adjacent comments #30

Closed
opened 2026-06-08 16:38:24 +00:00 by KofTwentyTwo · 0 comments
KofTwentyTwo commented 2026-06-08 16:38:24 +00:00 (Migrated from github.com)

Found during full-codebase review (see docs/review/01-write-path-safety.md).

Safety-critical: a false positive means apply deletes a real doc comment.

  1. [HIGH] internal/detect/detect.go (~line 290, 238): for line-comment file types, lineCommentRegion tolerates blank lines between comment lines, so "license header / blank / file doc comment" becomes ONE detected span. render.replaceSpan then deletes the doc comment on relicense/year-refresh. detect_test.go:554 enshrines this as intended.
  2. [MED] detect.go (~130, 339): SPDX-License-Identifier: is matched anywhere in the leading comment via strings.Index, so a comment that merely mentions the tag is treated as a managed header. The 'mere mention never qualifies' guarantee only holds for the fingerprint path.
  3. [LOW] block detection spans only the first /* */, so a two-block header is not fully de-stacked.

Fix: confine line-comment regions to contiguous comment lines; require the SPDX tag to sit on its own comment line in the managed region.

Found during full-codebase review (see docs/review/01-write-path-safety.md). Safety-critical: a false positive means `apply` deletes a real doc comment. 1. **[HIGH]** `internal/detect/detect.go` (~line 290, 238): for line-comment file types, `lineCommentRegion` tolerates blank lines between comment lines, so "license header / blank / file doc comment" becomes ONE detected span. `render.replaceSpan` then deletes the doc comment on relicense/year-refresh. `detect_test.go:554` enshrines this as intended. 2. **[MED]** `detect.go` (~130, 339): `SPDX-License-Identifier:` is matched anywhere in the leading comment via `strings.Index`, so a comment that merely mentions the tag is treated as a managed header. The 'mere mention never qualifies' guarantee only holds for the fingerprint path. 3. **[LOW]** block detection spans only the first `/* */`, so a two-block header is not fully de-stacked. Fix: confine line-comment regions to contiguous comment lines; require the SPDX tag to sit on its own comment line in the managed region.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: KofTwentyTwo/license-tool#30