Header detection over-matched on line-comment file types: lineCommentRegion
tolerated truly-blank lines between comment lines, so a layout of
"license header / blank line / unrelated file doc comment" collapsed into a
single detected span. render.replaceSpan then deleted that doc comment on
relicense or year-refresh. This is safety-critical -- a false positive in
detection means apply removes real source content.
Two fixes:
1. lineCommentRegion now stops at the first non-comment line, and a
truly-blank line ends the contiguous run. Our own rendered headers never
contain a truly-blank internal line: the renderer emits separator lines as
a bare comment prefix (e.g. "#" or "//"), which is still a comment line and
stays in-region. Confining the region to contiguous comment lines is
therefore byte-idempotent for headers we write, while refusing to absorb a
foreign comment that a blank line has separated from the header.
2. spdxIdentifierTag now requires the SPDX-License-Identifier tag to begin a
comment line (leading whitespace allowed) rather than matching anywhere via
strings.Index. A comment that merely mentions the tag mid-prose is
documentation, not a managed header, and must not qualify a foreign comment
for replacement.
When in doubt the detector now under-detects (leaves a foreign comment alone)
rather than over-detecting. Block-comment multi-block de-stacking (issue #30
LOW) is deferred: spanning only the first block is the safe, under-detecting
choice.
Idempotency and round-trip are preserved and verified by the existing
TestSpliceIdempotent / TestApplyFile* suites plus a new end-to-end applier
test that relicenses a line-comment file and asserts a doc comment below the
header's blank separator survives. The enshrining region test was updated to
require the corrected contiguous behavior with the safety rationale inline.
AtomicWrite used os.Stat + os.Rename with no symlink check, so writing a
managed LICENSE / LICENSES/<id>.txt that is a deliberate symlink (e.g.
LICENSE -> LICENSES/MIT.txt) would replace the link with a regular file,
silently breaking it.
AtomicWrite now lstats the target and refuses with errSymlinkTarget when it
is a symlink, leaving the link and its target untouched. The error message
contains "refusing to write" so the CLI's isWriteRefusal mapping classifies
it as a write refusal (exit code 3). writeManaged propagates this refusal as
a returned error (other write failures stay recorded per-file as before).
Closes#33
The spdxAliases table fabricated SPDX ids, violating the package's
"never guess" invariant:
- "bsd" / "bsd license" invented a clause count (could be 2/3/4-clause).
- "gnu lesser general public license" invented BOTH a version (2.1 vs 3.0)
and the grant ("only" vs "or-later").
These wrong guesses flowed into deny/allow verdicts and the incompatibility
table, where -only vs -or-later and clause counts are decisive. Such ambiguous
spellings now leave normalizeSPDX returning ok=false, so the caller emits an
unresolved dependency with a reason rather than a fabricated id. Only
unambiguous, exact spellings remain (explicit 3-clause BSD, Apache, MIT, ISC,
MPL-2.0, Unlicense, the explicit AGPL "v3.0 or later" spelling).
Also fixes the related resolved-but-unclassifiable contradiction: alias
re-validation now uses spdx.Lookup (curated rendering set) instead of
spdx.Validate (full index). The Eclipse Public License aliases (EPL-2.0 /
EPL-1.0) passed Validate but are outside the curated set, so policy.Classify
returned Unknown while the dependency was still marked resolved. They are now
left unresolved. Any future alias whose target is valid-but-uncurated is
structurally rejected by the Lookup gate.
Closes#31
Address findings from a three-agent adversarial review of the GH-35
delta:
- Compute the repo-wide incompatibility set from the full report, not the
--only-narrowed listing, so a filtered listing can no longer understate
a group's policy-aware risk. Add groupFilesWith(r, spec, incompatIDs)
and thread the full-report set through the text/markdown/JSON renderers.
- Use path.Base (not filepath.Base) on the slash-normalized enumerate
path when matching the config file, for cross-platform correctness.
- Tests: directory-grouping escalation across a mixed bucket, incompat
beating the headerless-unknown fallback, non-policy tokens not
escalating, incompatibleIDs cardinality (0/1/3 ids), the filtered-
listing risk contract, nested/--only config exclusion, no detected-
license/violation leak on the skipped config, and a check exit-code
regression guard for the config exclusion.
- Docs: correct the README grouped-output samples (risk markers,
(no-header) label, JSON group shape) and note the check exit-code
implication of the config exclusion in README and CHANGELOG.
A group's [risk] marker was computed solely from its files' license
categories, so it was policy-blind: an Apache-2.0 group beside an
AGPL-3.0-or-later group read "low" even though the pair is a curated
hard incompatibility, and a deny-listed permissive license also read
"low".
Escalate a group's risk to "high" when it carries a policy concern: a
file whose license is party to a repo-level hard incompatibility
(computed once over the distinct source ids via policy.Incompatible),
or a file carrying a file-scoped policy-violation token. Category-only
risk is preserved when there is no policy concern, with no false
escalation (MIT beside AGPL, not an incompatibility, stays "low").
The config file is YAML (commentable), so the audit counted it as a
coverable source file missing a managed header, inflating sourceTotal
and sourceMissing. It is tool metadata, not coverable source, and never
carries a header.
Mark any file whose basename is .license-tool.yaml as skipped (reason
"tool config") in the audit pipeline, so it still appears in the file
listing but is excluded from source/header tallies. Export
config.RepoConfigName as the single source of truth for the name.
Replaces the 7-step 3-pane wizard with a single repo-seeded form and a
persistent adaptive preview. wizard.go is now a thin Bubble Tea adapter;
all logic lives in internal/initwizard at 100% coverage.
Closes#29
james.maes
