KofTwentyTwo/license-tool
2
0 Fork 0
Code Issues 2 Pull Requests 9 Actions Packages Projects Releases 6 Wiki Activity

audit and check report controls are parsed but not honored #18

New Issue
Closed
opened 2026-06-04 16:19:22 +00:00 by KofTwentyTwo · 2 comments
KofTwentyTwo commented 2026-06-04 16:19:22 +00:00 (Migrated from github.com)
Copy link
Copy source

Finding

Report-related CLI controls are documented and partially parsed, but do not produce the advertised behavior. Audit ignores the output destination, check parses format and output controls but does not render a report, and the documented dependency-skip alias is missing.

Evidence

  • README.md:53 to README.md:55 documents JSON and Markdown output files plus a dependency-skip flag.
  • README.md:63 to README.md:68 says check forces machine-friendly output and supports fail conditions.
  • cmd/license-tool/commands.go:73 to cmd/license-tool/commands.go:80 stores format, output, dependency, resolver, and fail-on flags.
  • cmd/license-tool/commands.go:98 to cmd/license-tool/commands.go:107 always renders audit to stdout.
  • cmd/license-tool/commands.go:120 to cmd/license-tool/commands.go:142 calls report.Check and exits without rendering.
  • cmd/license-tool/commands.go:151 to cmd/license-tool/commands.go:157 binds only a positive dependency boolean, not the documented dependency-skip alias.

Reproduction

  1. Run audit with JSON format and an output path.
  2. Observe that JSON is printed to stdout and the output file is not created.
  3. Run check with JSON format on a passing tree.
  4. Observe no report output.
  5. Run audit with the documented dependency-skip alias.
  6. Observe a usage failure.

Why It Matters

Report-file workflows, CI artifacts, and machine-readable check output are part of the public CLI contract. Scripts depending on those options cannot currently use the tool as documented.

Suggested Fix

Centralize report rendering for audit and check. Honor output destinations, add the documented dependency-skip alias or remove it from docs, and add command tests for each advertised output path.

## Finding Report-related CLI controls are documented and partially parsed, but do not produce the advertised behavior. Audit ignores the output destination, check parses format and output controls but does not render a report, and the documented dependency-skip alias is missing. ## Evidence - `README.md:53` to `README.md:55` documents JSON and Markdown output files plus a dependency-skip flag. - `README.md:63` to `README.md:68` says `check` forces machine-friendly output and supports fail conditions. - `cmd/license-tool/commands.go:73` to `cmd/license-tool/commands.go:80` stores format, output, dependency, resolver, and fail-on flags. - `cmd/license-tool/commands.go:98` to `cmd/license-tool/commands.go:107` always renders audit to stdout. - `cmd/license-tool/commands.go:120` to `cmd/license-tool/commands.go:142` calls `report.Check` and exits without rendering. - `cmd/license-tool/commands.go:151` to `cmd/license-tool/commands.go:157` binds only a positive dependency boolean, not the documented dependency-skip alias. ## Reproduction 1. Run audit with JSON format and an output path. 2. Observe that JSON is printed to stdout and the output file is not created. 3. Run check with JSON format on a passing tree. 4. Observe no report output. 5. Run audit with the documented dependency-skip alias. 6. Observe a usage failure. ## Why It Matters Report-file workflows, CI artifacts, and machine-readable check output are part of the public CLI contract. Scripts depending on those options cannot currently use the tool as documented. ## Suggested Fix Centralize report rendering for audit and check. Honor output destinations, add the documented dependency-skip alias or remove it from docs, and add command tests for each advertised output path.
KofTwentyTwo commented 2026-06-04 16:29:28 +00:00 (Migrated from github.com)
Copy link
Copy source

Status: confirmed — all three sub-claims reproduce against the current tree (built dev, SPDX list 3dfd9aa): --output is silently ignored, check renders no report, and --no-deps is unbound.

Severity: high (matches the bug label, raised within bug's range). This is a broken-public-contract defect, not a crash: three documented CLI behaviors are non-functional and one (--no-deps) is a hard usage error (exit 2). No data loss, so not critical — but CI artifact and machine-readable-check workflows are unusable as documented.

Verified root cause (exact locations):

  • --output ignored (audit): cmd/license-tool/commands.go:bindAuditFlags:154 binds &f.output, but f.output is read nowhere — newAuditCmd's RunE at commands.go:107 hardcodes report.Render(cmd.OutOrStdout(), r, format). grep confirms line 154 is the only reference to f.output in the package.
  • check renders nothing: report.Check (internal/report/report.go:Check:377-386) returns only (exitCode int, err error) and never renders. The check RunE (commands.go:newCheckCmd:130-142) consumes only code, calls os.Exit(code) on non-zero, and returns — so --format/--output for check are parsed and discarded. Reproduced: check --format json emitted 0 bytes.
  • --no-deps missing: bindAuditFlags:155 binds only --deps (BoolVar, default true). There is no negative alias. Reproduced: audit --no-deps exits 2 (unknown flag).

Reproduction confirmed (temp dir, MIT/notice config):

  1. audit --format json --output audit.json . → exit 0, audit.json NOT created, JSON printed to stdout.
  2. check --format json . → 0 bytes of report output.
  3. audit --no-deps . → exit 2.

Suggested fix:

  • Centralize a writer-selection helper shared by audit and check: when f.output != "", open the file (os.Create) and render to it (close/error-handle); else cmd.OutOrStdout(). Apply at commands.go:107 (audit) and add an equivalent render step in check after the policy decision.
  • Make check actually render. report.Check currently throws away the model.Report. Cleanest path: have check call report.Audit for the report, render it (honoring --format/--output), then derive the exit code (extract the pass/fail logic from report.Check:382-385 into an exported predicate, or change Check to return (model.Report, int, error)). Keep the os.Exit(1) gate on failure.
  • --no-deps: prefer adding the alias over editing docs, since both README.md:55 and docs/license-tool-design.md:25 advertise --deps|--no-deps. cobra's BoolVar does not auto-generate a --no- form; register an explicit --no-deps (or use the --deps/--no-deps pair pattern) and reconcile it with the existing --deps default-true so the two cannot contradict.
  • Like the v0.2.1 universal-shebang fix touched both the render INSERT path and the detect boundary, this fix must touch both command paths (newAuditCmd and newCheckCmd) consistently — fixing only audit would leave check still silent. (No filetype table is involved here; that caveat applies to header-boundary bugs, not these CLI-wiring defects.)
  • 100% gate: add command-level tests in cmd/license-tool (or extend internal/report tests) covering: --output writes a file and stdout stays empty; check --format json emits a report on both pass and fail; --no-deps parses and disables dep resolution. The new file-open and writer-selection branches and the check render branch must each be exercised to keep .testcoverage.yml green.

Corrections to the issue: minor only.

  • The body calls --no-deps a "dependency-skip alias." It is documented as the negative half of the --deps|--no-deps pair (README:55, design doc:25), not strictly an "alias" — functionally equivalent, but the fix is "add the negative flag," not "alias an existing one."
  • The reproduction frames check's gap as "no report output" on a passing tree; the missing-render defect is unconditional (it never renders on pass or fail). My passing-config repro actually exited 1 due to policy, yet still produced 0 bytes — confirming the render gap is independent of pass/fail, slightly broader than the body implies.
  • Otherwise all line references (commands.go:73-80, 98-107, 120-142, 151-157) and the README references are accurate.

Open questions (maintainer must decide):

  • For check, should the rendered report always print, or only when --output/non-default --format is given? Today check is silent-by-design as a pure gate; rendering changes its stdout contract for existing CI consumers.
  • --no-deps vs --deps=false semantics: keep both and define precedence if both are passed, or replace the default-true --deps with the explicit pair?
  • Should --output failures (unwritable path) map to exit 4 (internal) or 2 (usage)? The documented exit-code table (README:113-121) doesn't cover this.

Overlaps: none found in the cited code. If a sibling issue tracks check's exit-code/output contract, this should be coordinated with it; I did not enumerate the full issue list.

Automated maintainer review.

**Status**: confirmed — all three sub-claims reproduce against the current tree (built `dev`, SPDX list `3dfd9aa`): `--output` is silently ignored, `check` renders no report, and `--no-deps` is unbound. **Severity**: high (matches the `bug` label, raised within `bug`'s range). This is a broken-public-contract defect, not a crash: three documented CLI behaviors are non-functional and one (`--no-deps`) is a hard usage error (exit 2). No data loss, so not critical — but CI artifact and machine-readable-check workflows are unusable as documented. **Verified root cause** (exact locations): - **`--output` ignored (audit):** `cmd/license-tool/commands.go:bindAuditFlags:154` binds `&f.output`, but `f.output` is read nowhere — `newAuditCmd`'s `RunE` at `commands.go:107` hardcodes `report.Render(cmd.OutOrStdout(), r, format)`. `grep` confirms line 154 is the only reference to `f.output` in the package. - **`check` renders nothing:** `report.Check` (`internal/report/report.go:Check:377-386`) returns only `(exitCode int, err error)` and never renders. The `check` `RunE` (`commands.go:newCheckCmd:130-142`) consumes only `code`, calls `os.Exit(code)` on non-zero, and returns — so `--format`/`--output` for `check` are parsed and discarded. Reproduced: `check --format json` emitted 0 bytes. - **`--no-deps` missing:** `bindAuditFlags:155` binds only `--deps` (`BoolVar`, default `true`). There is no negative alias. Reproduced: `audit --no-deps` exits 2 (unknown flag). **Reproduction confirmed** (temp dir, MIT/notice config): 1. `audit --format json --output audit.json .` → exit 0, `audit.json` NOT created, JSON printed to stdout. 2. `check --format json .` → 0 bytes of report output. 3. `audit --no-deps .` → exit 2. **Suggested fix**: - Centralize a writer-selection helper shared by `audit` and `check`: when `f.output != ""`, open the file (`os.Create`) and render to it (close/error-handle); else `cmd.OutOrStdout()`. Apply at `commands.go:107` (audit) and add an equivalent render step in `check` after the policy decision. - Make `check` actually render. `report.Check` currently throws away the `model.Report`. Cleanest path: have `check` call `report.Audit` for the report, render it (honoring `--format`/`--output`), then derive the exit code (extract the pass/fail logic from `report.Check:382-385` into an exported predicate, or change `Check` to return `(model.Report, int, error)`). Keep the `os.Exit(1)` gate on failure. - `--no-deps`: prefer adding the alias over editing docs, since both `README.md:55` and `docs/license-tool-design.md:25` advertise `--deps|--no-deps`. cobra's `BoolVar` does not auto-generate a `--no-` form; register an explicit `--no-deps` (or use the `--deps`/`--no-deps` pair pattern) and reconcile it with the existing `--deps` default-true so the two cannot contradict. - Like the v0.2.1 universal-shebang fix touched **both** the render INSERT path and the detect boundary, this fix must touch **both** command paths (`newAuditCmd` and `newCheckCmd`) consistently — fixing only `audit` would leave `check` still silent. (No filetype table is involved here; that caveat applies to header-boundary bugs, not these CLI-wiring defects.) - 100% gate: add command-level tests in `cmd/license-tool` (or extend `internal/report` tests) covering: `--output` writes a file and stdout stays empty; `check --format json` emits a report on both pass and fail; `--no-deps` parses and disables dep resolution. The new file-open and writer-selection branches and the `check` render branch must each be exercised to keep `.testcoverage.yml` green. **Corrections to the issue**: minor only. - The body calls `--no-deps` a "dependency-skip alias." It is documented as the negative half of the `--deps|--no-deps` pair (README:55, design doc:25), not strictly an "alias" — functionally equivalent, but the fix is "add the negative flag," not "alias an existing one." - The reproduction frames check's gap as "no report output" on a *passing* tree; the missing-render defect is unconditional (it never renders on pass or fail). My passing-config repro actually exited 1 due to policy, yet still produced 0 bytes — confirming the render gap is independent of pass/fail, slightly broader than the body implies. - Otherwise all line references (`commands.go:73-80`, `98-107`, `120-142`, `151-157`) and the README references are accurate. **Open questions** (maintainer must decide): - For `check`, should the rendered report always print, or only when `--output`/non-default `--format` is given? Today check is silent-by-design as a pure gate; rendering changes its stdout contract for existing CI consumers. - `--no-deps` vs `--deps=false` semantics: keep both and define precedence if both are passed, or replace the default-true `--deps` with the explicit pair? - Should `--output` failures (unwritable path) map to exit 4 (internal) or 2 (usage)? The documented exit-code table (README:113-121) doesn't cover this. **Overlaps**: none found in the cited code. If a sibling issue tracks `check`'s exit-code/output contract, this should be coordinated with it; I did not enumerate the full issue list. _Automated maintainer review._
KofTwentyTwo commented 2026-06-04 20:28:14 +00:00 (Migrated from github.com)
Copy link
Copy source

Resolved on develop in commit 742c124e6ec2afa9312075db3112bc2def3c91ff.

Validation:

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.
No Branch/Tag Specified
Labels
Clear labels
bug
Something isn't working
 dependencies
Pull requests that update a dependency file
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 github_actions
Pull requests that update GitHub Actions code
 go
Pull requests that update go code
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 wontfix
This will not be worked on
No labels bug
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
forge-recovery (Forge Recovery) james.maes (James Maes)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: KofTwentyTwo/license-tool#18