Skip to content

audit 2026-09-02: remediate all findings on the v0.5.34..v0.5.42 delta - #131

Merged
musharna merged 10 commits into
mainfrom
audit-2026-09-02-fixes
Sep 2, 2026
Merged

audit 2026-09-02: remediate all findings on the v0.5.34..v0.5.42 delta#131
musharna merged 10 commits into
mainfrom
audit-2026-09-02-fixes

Conversation

@musharna

@musharna musharna commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Remediates every finding of the 2026-09-02 four-lane audit of the v0.5.34..v0.5.42 delta (cwd-derived project identity, mcp 2.x migration, per-warning events, privacy scrub). 0 HIGH, 10 MED, ~18 LOW; all addressed. Each fix carries a test that was watched failing on the pre-fix code; the three race/floor guards were additionally mutation-verified (the test fails when its guard is reverted).

MED

  • Read-side project filter did not fold. job list --project and /events?project= compared the raw string while submit stored the folded, registered spelling, so a project split into two views and no single spelling returned all its jobs. Both now match {canonical, typed} on project plus typed on project_label. project_label gains an index, created by migrate() on in-place upgrades too.
  • Roots without priority were silently dropped. load_projects skipped entries lacking priority before roots validation ran, contradicting the documented "raise, don't drop" contract. Now a load error; _default may not declare roots either.
  • CITATION.cff release DOI was four releases stale (named the v0.5.38 Zenodo record beside version: 0.5.42). Only the concept DOI remains: a per-release DOI is stale by construction on the release commit, since Zenodo mints it afterwards.
  • MCP hid the substitution. jobd_list rows and the jobd_submit result now carry project_label; a missing argument (jobd_status {}) is an is_error result of kind invalid_arguments instead of a protocol-level crash; dispatch runs off the event loop so a wait=true submit no longer stalls the stdio session.
  • Docs read as unexecuted. Shipped-in-v0.5.42 banners on the design and plan, docs/projects-yaml.md retitled with historical sections marked, README link text fixed, new docs/events.md pinned two-way to KNOWN_EVENTS.
  • Test-suite guards that fail open. Corpus replay arm 2 gains a non-empty floor (it passed while checking zero rows under mutation); a parity test pins the replay's ROOTS table to config/projects.yaml; the ambiguous-fold branches in config.py are covered (a mutation picking the first match survived before); a guard asserts the suite imports jobd from src/ (a non-editable wheel in the venv had made local mutation checks vacuous). Coverage floor value lives in pyproject with a lint that ci.yml agrees.
  • Privacy. A real tailnet login line the chore: drop hardcoded personal paths from tracked files #121 scrub missed in the plan doc, and a hard-coded personal DB path in the corpus export script, are gone (the script now requires JOBD_DB). Rebased onto privacy: tracked config, corpus and docs carry no real paths or private names #129's pseudonymisation; new tests use its name map. Found during that rebase: privacy: tracked config, corpus and docs carry no real paths or private names #129's .gitignore rule for projects.local.yaml was one line of literal \n and matched nothing; the rule is fixed in the companion PR from that session, and this PR adds the git check-ignore test that would have caught it.

LOW

/reload under the projects mutation lock; leading // in roots and cwd collapsed; job projects set/nudge percent-encode the name (../reload used to run the config reload and then report a write); --explain names the typed spelling on a pure fold; GET /projects and job projects list show roots; the worker's reparented-orphan sweep gate is atomic with the scan (at max_concurrent > 1 a second job's fresh child could be SIGTERMed in the window); duplicate test removed; -rs on CI pytest; CodeQL scans actions; release.yml top-level read-only permissions.

Verification

Full suite with CI flags on the rebased branch: 1341 passed, 9 live-only deselected, 87.01% branch coverage (gate 83). mypy clean (48 files), ruff check and format clean. Audit record: ~/.claude/projects/-home-mjarnold-jobd/memory/jobd_audit_2026-09-02.md (local).

…ity is a load error

audit 2026-09-02 C-1, C-2, L-1, L-2, L-5.

- routes/jobs.py, routes/events.py: the read-side project filter compared
  the raw string while submit stored the folded, registered spelling, so a
  project split into two views and no spelling returned all its jobs.
  Both filters now match {canonical, typed} on project plus typed on
  project_label.
- config.py: the 'priority not in cfg -> continue' skip ran BEFORE
  _parse_roots, so a project declared with roots and no priority was
  dropped with no log line, contradicting docs/projects-yaml.md. Raise.
  _default may not declare roots (skipped by project_from_cwd by design).
  Leading '//' collapses onto '/' in roots and cwd; normpath preserves it.
- db.py: index project_label (list ORs on it); migrate() creates the same
  index on in-place upgrades. The pre-existing migration test dropped the
  column directly, which SQLite refuses once an index references it.
- routes/config.py: /reload swaps state under projects_mutation_lock so a
  concurrent set/nudge cannot persist from the swapped-in table.

9 new tests, each watched failing on the pre-fix code.
…ch off the event loop

audit 2026-09-02 Q-3 plus two LOWs.

- tools.py: _LIST_SUMMARY_FIELDS and the submit result carry project_label.
  The broker filter matches either name, so a list row could arrive under
  an identity the agent never typed with no field saying why.
- server.py: KeyError/TypeError from a tool (missing required argument)
  become an is_error result of kind invalid_arguments; under mcp 2.x the
  uncaught exception was a protocol-level 'Internal server error'.
  _dispatch runs via asyncio.to_thread; it drives a blocking httpx client
  and a wait=true submit could hold the loop for 270 s.
- schemas.py: the project description said 'falls back to _default'; the
  fallback is now the project whose roots contain cwd, then _default.

5 tests (4 new + 1 widened key-set pin), each watched failing first.
…names; list roots

audit 2026-09-02 LOWs (quality 6, 8; security 5).

- cli.py --explain: the 'submitted as' line lived only inside the
  matched_root branch, so a rule-1 fold never showed the typed spelling.
- cli.py projects set/nudge: quote(name, safe='') so '../reload' can only
  address /projects/<name> (it used to run the reload and then claim a
  write). Test drives the real Typer command against the real app and
  checks that the on-disk config edit did NOT become visible.
- broker/projects.py: _entry_to_yaml_dict emits roots (read surface only;
  _persist_projects writes priorities alone). cli.py list renders them.
- docs/projects-yaml.md: matched_root is surfaced in three places, not two
  (the dry-run plan's validation.effective_matched_root was undocumented).

4 new tests, each watched failing first.
…lision coverage

audit 2026-09-02 T-1, T-2, T-3 plus tests/CI LOWs.

- test_corpus_replay.py: arm 2 gains a non-empty floor counted in jobs
  (63 pairs / 1129 jobs measured); with canonical_project_name disabled it
  passed while checking zero rows -- the same fail-open shape as the 07-25
  route-table guard. New parity test pins ROOTS to config/projects.yaml
  (681f60b's drift can no longer recur silently).
- test_project_key_collisions.py: the ambiguous-fold branch and the
  load-time collision warning were executed by no test; a mutation picking
  the first match survived the suite. Both now killed (verified).
- test_deploy_lint.py: the suite must import jobd/job_cli from <repo>/src.
  A non-editable wheel in the venv made every local run test the installed
  snapshot (0% coverage, vacuous mutation checks). Coverage floor value
  now lives in pyproject [tool.coverage.report] and the lint checks ci.yml
  agrees.
- test_packaging.py: drop the duplicate server.json version test (the
  deploy lint already has it).
- ci.yml: -rs so always-skipped systemd-scope tests are visible.
  codeql.yml: scan the 'actions' language too. release.yml: top-level
  read-only permissions (publishing jobs keep their own blocks); v7.0.1
  comment drift fixed.
…a tailnet address

audit 2026-09-02 S-1, Q-1, Q-2, Q-4 plus docs LOWs.

- docs/plans/...-plan.md:1138 carried the maintainer's ssh login and
  tailnet IP (missed by #121's scrub, regressing the PL-3 decision that
  rested on none being in the published tree). Generic host now. The
  maintainer's DB path is gone from the plan, the design doc, and
  scripts/export_project_cwd_corpus.py, which now requires JOBD_DB.
- CITATION.cff: the per-release DOI named the v0.5.38 record beside
  version 0.5.42. It cannot be right on a release commit (Zenodo mints it
  afterwards), so only the concept DOI remains, with the reasoning inline.
- Design doc said 'approved, not yet planned'; plan had 44 unchecked boxes
  and no shipped marker; docs/projects-yaml.md was titled 'Plan' with stale
  line refs above the current §10 spec; README link text named a path that
  does not exist. Banners, retitle, and a historical-sections note.
- docs/events.md: the six event names added in the delta had no catalog.
  Every KNOWN_EVENTS name is now documented, pinned two-way by
  tests/test_events_catalog_doc.py (with a non-empty floor).
- config/projects.yaml: header states the decision to track the
  maintainer's real roots and what a stranger must replace; the jepagame
  comment no longer overclaims what the corpus shows.
… sweep

audit 2026-09-02 L-4. _is_solo_in_flight() was a point read released before
sweep_and_kill_reparented_orphans scanned /proc; with max_concurrent > 1
the poll loop could _register_in_flight(B) and Popen B in that window, and
B's child (ppid == worker, not in A's tracked set) was SIGTERMed as an
orphan. Registration already takes _in_flight_lock before any Popen, so
_sweep_reparented_orphans_if_solo holds that lock across check + scan; the
point-read gate is deleted. Real-threads test asserts no registration can
complete while the scan runs, and fails when the lock is released before
the scan (verified). The prior gate test is ported to the helper.
The audit branch was written against the pre-#129 tree; after the rebase this
one new file still named the real home directory and project, which
tests/test_no_private_paths.py now forbids.
Found while rebasing onto #129: its .gitignore rule was appended as ONE line
holding literal backslash-n sequences, so 'git check-ignore
config/projects.local.yaml' printed nothing and the real-roots overlay showed
as '??'. The rule itself is corrected in the companion privacy fix by the
session that owns #129; this adds the positive control that would have caught
it: a test asserting git check-ignore accepts the path (watched failing on
the broken rule).
@github-advanced-security

Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

The `jobd_list` summary shape was written down in three places: the
`_LIST_SUMMARY_FIELDS` tuple, the synthetic assertion in test_tools.py, and
the live-broker assertion in test_live.py. Adding `project_label` to the
source updated the first two; the third went stale and only surfaced in CI's
live leg, because the live suite is deselected on any machine without a
broker -- the copy that never runs locally is the copy that drifts.

Give the live test a module-level LIST_SUMMARY_KEYS and assert it equals
`_LIST_SUMMARY_FIELDS` from the default suite, so the next field added to the
source fails on a laptop instead of on a pull request.

Verified: removing project_label from LIST_SUMMARY_KEYS reproduces CI's exact
diff in the new guard. Live suite re-run against a real broker+worker
(RUN_LIVE_JOBD=1, loopback broker on 8799): 9 passed.
@musharna
musharna marked this pull request as ready for review September 2, 2026 19:15
@musharna
musharna merged commit d57daaf into main Sep 2, 2026
8 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.

2 participants