Fix broken wheel/sdist packaging, and harden CI (timeouts, Dependabot, root-only NSE job, workflow lint) - #42
Merged
Merged
Conversation
Port-script maps hardcoded f'{_DIR}/nse/<name>.nse', which only exists in a
checkout. Add _NSE_DIR (and the _resolve_nse_dir() helper behind it), which
falls back to {_DIR}/spoonmap_nse for the installed-wheel layout added in the
next commit, and repoint every occurrence at it. Checkout still wins when
nse/ is present, so existing path assertions are unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The wheel's [tool.hatch.build.targets.wheel] `artifacts` list was a no-op — it only un-excludes VCS-ignored files already matched by an include pattern — so the built wheel shipped spoonmap.py alone while _NSE_DIR pointed at a directory nothing packaged. Replace it with only-include plus force-include of nse/ -> spoonmap_nse/ (renamed to avoid a site-packages name collision). The default sdist swept in 54 of 91 entries of local session scratch (.remember/, .superpowers/sdd/, .claude/) because hatchling honours neither nested .gitignore nor .git/info/exclude; replace it with an explicit allowlist, which also deliberately omits ranges.txt and config.json since both hold real engagement data on an operator's working checkout. Add a `build` CI job that builds both artifacts and asserts on their actual contents: the sdist contains no scratch/engagement-data entries, the wheel contains every nse/*.nse file (count derived from the directory listing, not hardcoded), and a wheel installed into a clean venv resolves every bundled NSE path in both port-script maps to a real file on disk. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
uvx --from "bandit[toml]==1.9.4" pinned bandit itself but let its transitive dependencies float, contradicting the reasoning that put ruff exact-pinned in dev. Add bandit[toml]==1.9.4 to dev and refresh uv.lock so CI can invoke it via `uv run --frozen bandit`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test_resolver_prefers_nse_subdir_when_present duplicated the coverage already provided by test_nse_dir_prefers_checkout_directory just above it. More importantly, CI's wheel-contents check only walks INTERNAL_PORT_SCRIPTS/EXTERNAL_PORT_SCRIPTS, so a revert of any of the ~45 other _NSE_DIR call sites in _FINDING_REPRO or _scan_extra_sql_ports() back to a _DIR-relative nse/ path (which breaks once installed from the wheel) would be invisible to the suite. Add a test that asserts on the module's own source text instead, so every call site is covered at once regardless of which one regresses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Add timeout-minutes to every job (10 min; generous but bounded versus the 6-hour default). This suite exercises work_queue.join(), threading.Event polling, and KeyboardInterrupt handling, and a past bug in that area failed as a hang rather than a clean failure. - Add nse-root: runs tests/test_nse_integration.py as root via `sudo -E env "PATH=$PATH" uv run --frozen pytest ... --no-cov`, so TestOpenvpnDetectNseUdp (the -sU path, gated on os.geteuid() == 0 and skipped everywhere else) actually executes. sudo resets PATH, which drops the uv setup-uv just installed — verified locally that the explicit PATH re-injection is what makes this work rather than silently erroring or skipping. Coverage is disabled for this one job (95% floor in pyproject.toml is unchanged) since a single-module run can't meet a whole-suite floor. - Install nmap on macOS (brew) as well as Ubuntu (apt), and drop the Linux-only guard: until now all 26 NSE integration tests silently skipped on macOS, the platform the tool is developed on. Add a nmap-presence guard step to every job that expects it, so a broken install fails loudly instead of turning assertions into skips. - Scope concurrency's cancel-in-progress to pull_request events, so a fast follow-up merge to main no longer cancels the previous commit's in-progress run and erases its green check. - Move bandit invocation to `uv run --frozen bandit` (dependencies now locked in pyproject.toml's dev group) and consolidate `uv lock --check` into the lint job instead of running it on every test matrix leg. - Add -rs to pytest invocations so skips are visible in the log. - Add workflow-lint: actionlint (YAML/expression errors) and zizmor (Actions-specific security auditing) against this file itself, since it's hand-edited often and carries several hand-maintained SHA pins. - Add .github/dependabot.yml: weekly github-actions and uv updates, so the hand-maintained SHA/version pins actually receive proposed security bumps. - Carry-over fixes from Task 1's review: add an empty-input guard to the wheel NSE-contents check (mirroring the clean-venv step's existing guard), drop the unused `import spoonmap` in the clean-venv snippet, and comment that only ranges.txt exercises the sdist assertion's six banned patterns today (the other five guard against a future commit). Verified locally: actionlint and zizmor both clean, `bash -n` on every shell block, YAML parses, and `sudo -E env "PATH=$PATH" uv run --frozen pytest tests/test_nse_integration.py -k OpenvpnDetectNseUdp` passes (not skips) both tests in TestOpenvpnDetectNseUdp. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both documents described exactly four jobs (test, test-legacy, lint, bandit); Task 1 had already added build without updating either. Bring both current: build, nse-root, and workflow-lint, plus the cancel-in-progress scoping, dependabot.yml, the single uv lock --check, and bandit's move into the locked dev group. Also correct README's "Python 3.6+ is required" — requires-python has been >=3.8 and CI floors there too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…inor
Important: nse-root previously exited 0 whether or not the root-gated
class actually ran, because both a genuine port conflict
(tests/conftest.py) and the os.geteuid() root gate
(tests/test_nse_integration.py) are pytest skips, not errors. Add:
- a preflight step running the exact same `sudo -E env "PATH=$PATH"
uv run --frozen ...` invocation to assert os.geteuid() == 0, proving
sudo/PATH-reinjection/uv resolution work before the real run;
- --junitxml on the real run, and a step that parses it for any
<skipped> whose message matches the root-gate reason string
specifically ("requires root for -sU raw socket scan"), distinct
from the port-conflict reason from conftest.py. A port-conflict skip
still passes; a root-gate skip now fails the job.
Verified both directions locally: passes when TestOpenvpnDetectNseUdp
runs as root (23 passed, 3 skipped — the 3 are the unrelated CUPS
port-631 conflict), and fails when the same class is skipped for the
root reason (non-root run, both tests skip, script exits 1 naming
them).
Minor fixes:
- nse-root: enable-cache: false (root-owned uv cache entries would
EPERM the runner-owned cache prune setup-uv runs afterward).
- workflow-lint comment: "four hand-maintained SHA pins" was already
stale (13 occurrences, 2 distinct SHAs); reworded to not need
updating as jobs are added.
- dependabot.yml: uv updates now scoped via `allow: dependency-type:
development`, matching that every locked dependency today lives in
the dev group.
- Documented why actionlint/zizmor stay on a bare `uvx --from` instead
of the locked dev group: neither is part of spoonmap's dependency
surface, both are single dependency-light binaries, and their blast
radius is limited to linting the workflow file, unlike bandit which
runs against spoonmap.py itself.
- workflow-lint now points actionlint (no path arg, auto-discovers)
and zizmor (.github/workflows/ as input) at the whole workflows
directory instead of naming ci.yml, so a future workflow file is
covered. dependabot.yml is deliberately excluded from both — neither
tool understands it, and its YAML validity is a separate parse.
- brew install nmap now sets HOMEBREW_NO_AUTO_UPDATE=1 so an
unrelated `brew update` can't eat into the 10-minute timeout.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, nse/ leak
Critical: workflow-lint had no `Install uv` step, unlike every other job.
`uv`/`uvx` is not preinstalled on ubuntu-latest, so both its steps died at
"command not found" on every real CI run while passing in local
verification, where `uv` was already on the developer's PATH — meaning
actionlint and zizmor audited nothing. Added the same pinned setup-uv step
the other six jobs use, with enable-cache: true (nothing in this job runs
under sudo, so there's no root-owned-cache conflict the way nse-root has).
Important: nse-root's junit guard was negative-only ("no skip mentioned the
root-gate reason") and passed vacuously if that reason string was reworded,
TestOpenvpnDetectNseUdp was renamed/deleted, or the whole module skipped
itself (nmap missing) — none of those produce a matching skip message.
Replaced with a positive assertion: at least one testcase with that
classname must exist in the junit report, and none of its testcases may be
skipped, for any reason. Verified all three previously-vacuous directions
now fail for real (non-root run: both tests skip and script exits 1 naming
them; class excluded via -k: "no testcase found", exit 1; a junit report
with zero collected testcases: same), and that a real root run (23 passed,
3 skipped — the 3 are the unrelated CUPS port-631 conflict on another
class) still passes.
Important: nse/ is a directory-wide include in both the sdist
(pyproject.toml) and the wheel's force-include, so anything sitting in it
ships regardless of name — reproduced a `nse/operator-notes.txt` landing in
both dist/*.tar.gz and dist/*.whl. Rewrote both the sdist and wheel content
checks to assert set equality against `git ls-files nse/`, not
`os.listdir('nse')` — the latter is tautological, since the build reads the
same working tree the check would compare against, so it can never catch
an untracked file. Verified: clean tree passes; with the untracked file
present, both checks now fail identifying `operator-notes.txt` as the
offender; removing it restores a clean pass.
Minor: added a positive floor to the sdist check — every required top-level
entry (spoonmap.py, nse/, tests/, README.md, CLAUDE.md, config.json.sample,
exclusions.txt, pyproject.toml, uv.lock, .bandit-baseline.json,
.github/workflows/) must actually be present. Deliberately hardcoded rather
than read back out of pyproject.toml's own include list: the failure this
guards against is that include list itself being gutted, so the floor can't
be derived from the same config it exists to check. Verified: temporarily
dropping "tests/" and "uv.lock" from pyproject.toml's include list makes
`uv build` still succeed but the new check correctly reports them missing;
restored afterward.
Minor: force-included config.json.sample into the wheel (pyproject.toml).
spoonmap.py's own config-error messages ("See config.json.sample for the
expected keys") name it by bare filename; it already shipped in the sdist
but not the wheel, leaving a wheel-installed user's error message pointing
at a file that didn't exist. exclusions.txt deliberately stays out: its
only reference is an interactive prompt's suggested default path, not a
template any error message points at, and the checked-in file is empty, so
shipping it adds no content.
Minor: doc drift — README.md and CLAUDE.md both said "the workflow file"
(singular) for workflow-lint; both tools now scan the whole
.github/workflows/ directory. Both docs now also describe the nse-root
junit guard's positive-assertion property (previously undocumented and,
after this fix, the job's single most important behavior) and the
Install-uv fix.
Minor: narrowed test_no_call_site_reverts_to_dir_relative_nse_path from a
substring regex over inspect.getsource() (which a future comment or
docstring quoting the banned pattern as prose could trip) to an AST walk
that matches only the code shape a real revert produces: a JoinedStr whose
FormattedValue names `_DIR`, immediately followed by a Constant string
starting with '/nse/'. Comments aren't part of the AST at all, and a plain
string constant (docstring or otherwise) can't produce that JoinedValue
shape, so prose can't trip it — verified locally with a comment, a
docstring, and a real `f'{_DIR}/nse/x.nse'` revert: only the last one is
flagged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Two packaging defects, found by actually building the artifacts rather than reading the config:
spoonmap.pyand nothing else. The[tool.hatch.build.targets.wheel] artifactslist was a silent no-op — hatchling'sartifactskey only un-excludes VCS-ignored files that already match an include pattern, it never adds files. Meanwhilespoonmap.pybuilt its NSE script paths from the directory holding the module, so apip installedspoonmaphanded nmap nonexistent paths for ~14 ports. The[project.scripts]entry point was a claimed interface nothing verified..remember/,.superpowers/sdd/,.claude/— plusranges.txtand any localconfig.json. Those are git-ignored via nested.gitignorefiles and.git/info/exclude, and hatchling honours neither. Auv build && uv publishfrom a working checkout would have shipped session logs and engagement scope.Plus CI gaps: no
timeout-minuteson any job (the suite exerciseswork_queue.join()andKeyboardInterruptpaths, and a past bug there failed as a hang), SHA-pinned actions with nothing to ever propose a bump, the macOS job never installing nmap, and the root-gated-sUNSE path verified nowhere.Packaging
_NSE_DIRresolver:{_DIR}/nsein a checkout,{_DIR}/spoonmap_nseonce installed. Checkout wins when both resolve. Verified across four deployment shapes including symlink-into-checkout, which works because_DIRusesrealpath.force-includeactually packages the NSE tree; the sdist gets an explicit allowlist, not a blocklist, so it fails closed against scratch directories that don't exist yet.buildjob asserts on real archive contents: sdist excludes scratch and contains every required entry, the wheel'sspoonmap_nse/matches git-trackednse/by set equality (an untracked.nsefile fails the build), and the wheel installed into a clean venv resolves every NSE path on disk.CI
timeout-minuteson all 7 jobs; Dependabot forgithub-actionsanduv.950 passed, 26 skippedagainst Ubuntu's968 passed, 8 skipped, i.e. every NSE integration test silently skipping on the platform the tool is developed on.nse-rootjob runstest_nse_integration.pyas root, the only placeTestOpenvpnDetectNseUdp(the-sUpath) executes. It carries a positive assertion that the class actually ran: both the root gate andconftest.py's port gate produce pytest skips, so the job would otherwise exit 0 having verified nothing. Verified failing when the class skips, when it's renamed, and when the module skips — plain pytest exits 0 in all three cases.concurrencycancellation scoped topull_request, so a fast follow-up merge no longer erases the previousmaincommit's green check.devgroup (uvx --from "pkg==X"pins the tool but floats its transitive deps);actionlint+zizmorlint the whole workflow directory.uv lock --checkruns once instead of five times.Verification
975 passed, 5 skipped, 100% coverage (floor 95%) · ruff clean ·uv lock --checkpasses · bandit 32 baseline / 0 new · actionlint clean · zizmor no findings. No suppressions, no baseline additions, coverage floor untouched.Both vacuous-pass guards were verified in the failing direction, not just the passing one — including planting an untracked
nse/operator-notes.nse, confirming hatchling does package it, and confirming the set-equality check catches it.Known residuals
nse-roothas never run on anubuntu-latestrunner; this PR's own CI is its first real test. It now fails loudly rather than passing empty.nse/leak protection is a CI gate, not a build-time exclusion: a localuv build && uv publishwith untracked scratch innse/would still leak. Closing that needs a hatch build hook — follow-up, not a blocker.cupsdon 631, the other needs 631 free). Deliberately not "fixed": the cause was inferred, not observed.-rsis now on every pytest invocation, so this PR's CI log prints the real reason and settles it.🤖 Generated with Claude Code