license command bypasses write safety and ignores commit options #12

New Issue

2026-06-04T16:19:15Z

KofTwentyTwo commented

2026-06-04 16:19:15 +00:00

(Migrated from github.com)

Finding

The license subcommand writes managed license files through ManageLicenseFiles directly, while apply goes through the write-safety gate first. The same command also accepts the commit-related flags through shared apply flag binding, but never passes commit intent to a commit path.

Evidence

cmd/license-tool/commands.go:214 to cmd/license-tool/commands.go:228 calls applier.ManageLicenseFiles directly.
internal/applier/applier.go:67 to internal/applier/applier.go:72 gates writes only inside Apply.
internal/applier/applier.go:281 to internal/applier/applier.go:300 defines the dirty-tree and non-git write refusal that license bypasses.
internal/applier/applier.go:188 to internal/applier/applier.go:238 writes managed license files when write mode is enabled.

Reproduction

Create a temporary non-git directory with a valid .license-tool.yaml.
Run license-tool license <dir> with the write flag.
Observe that LICENSE and LICENSES/<id>.txt are created even though apply would refuse a non-git write without force.
Repeat in a clean git repository with the write and commit flags.
Observe that the files are written, but no commit is created.

Why It Matters

The documented safety contract says write operations require a clean git tree unless explicitly overridden, and non-git directories require force. license is a write command and should honor the same contract. The accepted commit flags also create a false sense that the write was captured atomically.

Suggested Fix

Route license through the same write gate as apply, or move the gate into the managed-file writer. Either reject commit flags for license or implement the same scoped commit behavior as apply.

## Finding The `license` subcommand writes managed license files through `ManageLicenseFiles` directly, while `apply` goes through the write-safety gate first. The same command also accepts the commit-related flags through shared apply flag binding, but never passes commit intent to a commit path. ## Evidence - `cmd/license-tool/commands.go:214` to `cmd/license-tool/commands.go:228` calls `applier.ManageLicenseFiles` directly. - `internal/applier/applier.go:67` to `internal/applier/applier.go:72` gates writes only inside `Apply`. - `internal/applier/applier.go:281` to `internal/applier/applier.go:300` defines the dirty-tree and non-git write refusal that `license` bypasses. - `internal/applier/applier.go:188` to `internal/applier/applier.go:238` writes managed license files when write mode is enabled. ## Reproduction 1. Create a temporary non-git directory with a valid `.license-tool.yaml`. 2. Run `license-tool license <dir>` with the write flag. 3. Observe that `LICENSE` and `LICENSES/<id>.txt` are created even though `apply` would refuse a non-git write without force. 4. Repeat in a clean git repository with the write and commit flags. 5. Observe that the files are written, but no commit is created. ## Why It Matters The documented safety contract says write operations require a clean git tree unless explicitly overridden, and non-git directories require force. `license` is a write command and should honor the same contract. The accepted commit flags also create a false sense that the write was captured atomically. ## Suggested Fix Route `license` through the same write gate as `apply`, or move the gate into the managed-file writer. Either reject commit flags for `license` or implement the same scoped commit behavior as `apply`.

KofTwentyTwo commented

2026-06-04 16:29:37 +00:00

(Migrated from github.com)

Status: confirmed — the license subcommand calls applier.ManageLicenseFiles directly, which has no write-safety gate and no commit path, while accepting --commit/--commit-message that it never honors.

Severity: high (issue is labeled bug, no severity label). Two distinct safety-contract violations in a write command: (1) silent bypass of the dirty-tree / non-git refusal, and (2) silently accepting commit flags that are dropped. Both are confirmed reproducible and can mutate a repo in states the tool elsewhere refuses. Not critical (no data loss/corruption; writes are atomic and the content itself is correct), but it breaks the documented write contract, so high.

Verified root cause:

cmd/license-tool/commands.go:newLicenseCmd (RunE), lines ~223-228: builds applier.Options{Write, AllowDirty, Force, ManageLicenseFile: true} and calls applier.ManageLicenseFiles(path, cfg, ...) directly — omitting Commit/CommitMessage and never invoking any gate.
internal/applier/applier.go:ManageLicenseFiles, lines 189-208: no gateWrite call and no commit step — it only looks up the license, renders, and calls writeManaged for each file.
By contrast internal/applier/applier.go:Apply, lines 73-77 calls gateWrite when opts.Write, and lines 144-149 perform the optional commit when opts.Write && opts.Commit. gateWrite itself lives at lines 281-300.
Flag binding: cmd/license-tool/commands.go:bindApplyFlags, lines 251-252 registers --commit/--commit-message for both apply and license, so license advertises commit support it does not wire through.

Reproduction (built from current tree at HEAD):

Non-git dir with a valid .license-tool.yaml: license . --write (no --force) → exit 0, writes LICENSE + LICENSES/MIT.txt. The equivalent apply . --write exits 2 ("refusing to write in a non-git directory without --force"). Confirms the gate bypass.
Clean git repo: license . --write --commit → exit 0, files written, but git rev-list --count HEAD is unchanged and the two new files are left untracked. Confirms commit intent is silently dropped.

Suggested fix: Move the safety gate into the managed-file writer so it cannot be bypassed by callers — i.e. call gateWrite(path, opts) at the top of ManageLicenseFiles under if opts.Write (mirroring Apply at applier.go:73-77). Centralizing the invariant in the writer (rather than re-gating at each call site) follows the v0.2.1 universal-shebang precedent, where the fix was applied at the shared boundary rather than per-caller; here the analogous "both boundaries" are the two write entry points (Apply and ManageLicenseFiles) — putting the gate in ManageLicenseFiles also covers Apply's license-file management without double-gating, since Apply already gated once up front (so guard against re-gating, e.g. only gate in the path that isn't reached via Apply, or make gateWrite idempotent). For commit: either (a) reject --commit/--commit-message when used with license (cleanest if commit-on-license is out of scope), or (b) thread Commit/CommitMessage through and add the same opts.Write && opts.Commit commit block after the writes. Note the 100% coverage gate (.testcoverage.yml, total: 100): any new branch (the gate refusal, the commit block, or a flag-rejection error) needs a covering test in the applier/commands test suites, and the existing manageLicenseFilesFn/gateWrite seams can be reused to drive failure branches deterministically.

Corrections to the issue: minor. (1) The body cites the gate as internal/applier/applier.go:281–300; that range is correct for gateWrite, but the "gates writes only inside Apply" claim is at lines 73-77, not 67-72 (67-72 is the detectFn seam comment) — the gate call is a few lines below the cited range. (2) The body says "the documented safety contract says write operations require a clean git tree unless explicitly overridden" — accurate, but note the in-git default refuses a dirty tree (overridable with --allow-dirty); the non-git case is the --force path. Both reproduced. Otherwise the finding, evidence line refs, and impact are accurate.

Open questions (maintainer must decide):

Commit semantics for license: reject the flags, or implement a scoped commit? If implemented, should it be a separate conventional-commit message from apply's, and should license honor --commit-message the same way?
Gate placement: gate inside ManageLicenseFiles (covers both entry points but risks double-gating via Apply) vs. gate at the license command site only. Preference affects how the no-double-gate guard is written.
Should license --write in a non-git dir be allowed at all with --force, mirroring apply, or is managing a top-level LICENSE outside a repo simply unsupported?

Overlaps: none identified from this issue's content. A maintainer scanning open issues should check whether any other report covers ManageLicenseFiles gating or apply/license flag-binding divergence, but no specific issue number is implicated here.

Automated maintainer review.

**Status**: confirmed — the `license` subcommand calls `applier.ManageLicenseFiles` directly, which has no write-safety gate and no commit path, while accepting `--commit`/`--commit-message` that it never honors. **Severity**: high (issue is labeled `bug`, no severity label). Two distinct safety-contract violations in a write command: (1) silent bypass of the dirty-tree / non-git refusal, and (2) silently accepting commit flags that are dropped. Both are confirmed reproducible and can mutate a repo in states the tool elsewhere refuses. Not critical (no data loss/corruption; writes are atomic and the content itself is correct), but it breaks the documented write contract, so high. **Verified root cause**: - `cmd/license-tool/commands.go:newLicenseCmd` (RunE), lines ~223-228: builds `applier.Options{Write, AllowDirty, Force, ManageLicenseFile: true}` and calls `applier.ManageLicenseFiles(path, cfg, ...)` directly — omitting `Commit`/`CommitMessage` and never invoking any gate. - `internal/applier/applier.go:ManageLicenseFiles`, lines 189-208: no `gateWrite` call and no commit step — it only looks up the license, renders, and calls `writeManaged` for each file. - By contrast `internal/applier/applier.go:Apply`, lines 73-77 calls `gateWrite` when `opts.Write`, and lines 144-149 perform the optional commit when `opts.Write && opts.Commit`. `gateWrite` itself lives at lines 281-300. - Flag binding: `cmd/license-tool/commands.go:bindApplyFlags`, lines 251-252 registers `--commit`/`--commit-message` for both `apply` and `license`, so `license` advertises commit support it does not wire through. **Reproduction** (built from current tree at HEAD): - Non-git dir with a valid `.license-tool.yaml`: `license . --write` (no `--force`) → exit 0, writes `LICENSE` + `LICENSES/MIT.txt`. The equivalent `apply . --write` exits 2 ("refusing to write in a non-git directory without --force"). Confirms the gate bypass. - Clean git repo: `license . --write --commit` → exit 0, files written, but `git rev-list --count HEAD` is unchanged and the two new files are left untracked. Confirms commit intent is silently dropped. **Suggested fix**: Move the safety gate into the managed-file writer so it cannot be bypassed by callers — i.e. call `gateWrite(path, opts)` at the top of `ManageLicenseFiles` under `if opts.Write` (mirroring `Apply` at applier.go:73-77). Centralizing the invariant in the writer (rather than re-gating at each call site) follows the v0.2.1 universal-shebang precedent, where the fix was applied at the shared boundary rather than per-caller; here the analogous "both boundaries" are the two write entry points (`Apply` and `ManageLicenseFiles`) — putting the gate in `ManageLicenseFiles` also covers `Apply`'s license-file management without double-gating, since `Apply` already gated once up front (so guard against re-gating, e.g. only gate in the path that isn't reached via `Apply`, or make `gateWrite` idempotent). For commit: either (a) reject `--commit`/`--commit-message` when used with `license` (cleanest if commit-on-license is out of scope), or (b) thread `Commit`/`CommitMessage` through and add the same `opts.Write && opts.Commit` commit block after the writes. Note the 100% coverage gate (`.testcoverage.yml`, total: 100): any new branch (the gate refusal, the commit block, or a flag-rejection error) needs a covering test in the applier/commands test suites, and the existing `manageLicenseFilesFn`/`gateWrite` seams can be reused to drive failure branches deterministically. **Corrections to the issue**: minor. (1) The body cites the gate as `internal/applier/applier.go:281`–`300`; that range is correct for `gateWrite`, but the "gates writes only inside Apply" claim is at lines 73-77, not 67-72 (67-72 is the `detectFn` seam comment) — the gate call is a few lines below the cited range. (2) The body says "the documented safety contract says write operations require a clean git tree unless explicitly overridden" — accurate, but note the in-git default refuses a *dirty* tree (overridable with `--allow-dirty`); the non-git case is the `--force` path. Both reproduced. Otherwise the finding, evidence line refs, and impact are accurate. **Open questions** (maintainer must decide): - Commit semantics for `license`: reject the flags, or implement a scoped commit? If implemented, should it be a separate conventional-commit message from `apply`'s, and should `license` honor `--commit-message` the same way? - Gate placement: gate inside `ManageLicenseFiles` (covers both entry points but risks double-gating via `Apply`) vs. gate at the `license` command site only. Preference affects how the no-double-gate guard is written. - Should `license --write` in a non-git dir be allowed at all with `--force`, mirroring `apply`, or is managing a top-level `LICENSE` outside a repo simply unsupported? **Overlaps**: none identified from this issue's content. A maintainer scanning open issues should check whether any other report covers `ManageLicenseFiles` gating or `apply`/`license` flag-binding divergence, but no specific issue number is implicated here. _Automated maintainer review._

KofTwentyTwo commented

2026-06-04 20:28:06 +00:00

(Migrated from github.com)

Resolved on develop in commit 742c124e6ec2afa9312075db3112bc2def3c91ff.

Validation:

CI: success, https://github.com/KofTwentyTwo/license-tool/actions/runs/26977377097
CodeQL: success, https://github.com/KofTwentyTwo/license-tool/actions/runs/26977376421
Coverage gate: 100% (2087/2087)

Closing as fixed.

Resolved on `develop` in commit `742c124e6ec2afa9312075db3112bc2def3c91ff`. Validation: - CI: success, https://github.com/KofTwentyTwo/license-tool/actions/runs/26977377097 - CodeQL: success, https://github.com/KofTwentyTwo/license-tool/actions/runs/26977376421 - Coverage gate: 100% (2087/2087) Closing as fixed.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/license-tool#12