Skip to content

fix(cli): preserve registry inputs when writing reports - #758

Merged
rng1995 merged 1 commit into
NVIDIA:mainfrom
YMuskrat:ymuskrat/registry-output-guard
Oct 14, 2026
Merged

rng1995 merged 1 commit into
NVIDIA:mainfrom
YMuskrat:ymuskrat/registry-output-guard

Conversation

@YMuskrat

@YMuskrat YMuskrat commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #756.

A registry scan such as skillspector scan registry.json --mcp-registry --format json --output registry.json currently replaces its input with the generated report. The registry branch returns before the existing same-file guard. A comparison report can also be overwritten when passed as both --mcp-registry-compare and --output.

Extract the existing destination check into a shared helper and apply it to both registry inputs before scanning. Existing Path.samefile() behavior covers relative paths, symlinks and hardlinks; distinct output files continue to work.

Validation: make lint, make format-check, and make test pass (9,966 unit tests and 124 integration tests). Regression coverage checks all six input aliases, comparison-input protection, unchanged input bytes, and that rejected scans never call the scanner.

Reject output aliases of registry and comparison inputs before scanning.

Signed-off-by: YMuskrat <101849520+YMuskrat@users.noreply.github.com>

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

@YMuskrat thank you for spotting that registry scans skip the same-file output guard, and for extracting the existing check into a shared helper instead of copying it.

Value and readiness

Ready for final maintainer review. On main, skillspector scan registry.json --mcp-registry --format json --output registry.json returns from the registry branch before the same-file guard runs, then replaces the input with the report. --mcp-registry-compare previous.json --output previous.json destroys the comparison report the same way. The PR moves the guard into _validate_report_destination and calls it for the registry input and the comparison report before scan_registry runs. It keeps the existing semantics (is_file() + exists() + samefile()), so relative paths, parent traversal, symlinks, symlinked parents and hard links are all rejected. The ValueError is caught by the registry branch's existing except Exception and exits 2 before any scan or write. URL registry inputs are unaffected because Path(url).is_file() is false. The local-skill path calls the same helper with unchanged behavior.

Material findings

None.

PIC tradeoffs

None identified.

Verification and gaps

  • Read the scan() registry branch and the local-input guard on main b07f181d and on the merge of this head with main. git merge-tree is clean, and the helper is called at all three sites after the merge.
  • Tests: the existing alias test now also runs in registry mode (six aliases, JSON). It asserts exit 2, the error text, unchanged input bytes, and scan_registry.assert_not_called(). A new test covers the comparison report via same path and hard link. The existing success test now asserts the payload is untouched when the output is a different file. Each rejection case would fail on main, where the registry branch never reaches the guard.
  • CI is green on 1ac08a2b (6/6). The branch is about 220 commits behind main. Main changed cli.py in that time but not these blocks, so I expect no semantic drift; a branch update and fresh CI before merge would confirm it. I did not run the tests (static review).

Decision: Approved (reviewed head 1ac08a2b25699f2ff78ed8038f13e8c30dab3626)

@rng1995
rng1995 merged commit 73a4a00 into NVIDIA:main Oct 14, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Registry scans can overwrite their input or comparison report

2 participants