Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,19 @@ and could stop an installed copy from updating.

## [Unreleased]

### Fixed

- **hull's scanner registry no longer leaks per-scanner facts into the
scaffolder, and its goldens all regenerate.** The BASE==HEAD advisory and
the license-free remedy the organization blocker offers are now derived from
`ScannerSpec` instead of naming `trufflehog` in `scaffold.py`, so a third
scanner gets an advisory channel and is offered as a remedy automatically;
`sync_action_pins.py` regenerates all three hull goldens (not just one) with
a loud guard for an unmapped golden; the scan step copies its `with:` inputs
before rendering, matching how it already copies `env`; and the README names
trufflehog's manual bump path, since Dependabot cannot see a pin that appears
in no committed workflow (#30).

## [0.8.0] - 2026-07-22

### Added
Expand Down
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ The action pins are authored here:
| `actions/checkout` | `plugins/rigging/rigging/plan.py`, `plugins/hull/hull/plan.py` |
| `actions/setup-python`, `actions/setup-node` | `plugins/rigging/rigging/stacks.py` |
| `gitleaks/gitleaks-action` | `plugins/hull/hull/scanners.py` |
| `trufflesecurity/trufflehog` | `plugins/hull/hull/scanners.py` |

Every ref is pinned to a full commit SHA with a trailing `# v7`-style
comment. The SHA is the pin; the comment is for humans and for Dependabot.
Expand All @@ -319,6 +320,14 @@ python3 -m pytest -q
git commit -am "chore(deps): sync action pins into the registries"
```

One pin is the exception: `trufflesecurity/trufflehog`. Dependabot only bumps
refs that appear in a workflow committed to *this* repo, and shipyard's own
`security.yml` uses `gitleaks` — so trufflehog is shipped to consumers but
never dogfooded here, and Dependabot never proposes a bump for it. When a new
trufflehog release ships, edit `action_ref` and `action_ref_version` on the
`"trufflehog"` entry in `plugins/hull/hull/scanners.py` by hand, then run
`scripts/sync_action_pins.py` to regenerate the goldens.

The changelog gate also applies to `dependabot/*` branches (they classify
as `other` under trunk), so such a PR needs a `CHANGELOG.md` entry like any
other change.
Expand Down
450 changes: 450 additions & 0 deletions docs/superpowers/plans/2026-07-23-hull-scanner-registry-cleanup.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
# hull scanner-registry cleanup (#30) — design

**Status:** approved for planning
**Issue:** [#30](https://github.com/submtd/shipyard/issues/30) — "hull: the
trufflehog pin has no automated bump path, and two scanner facts are hardcoded
outside the registry"

Four items deliberately deferred from the #27 review. None block other work;
all are the kind of latent inconsistency that bites a maintainer later. Each is
independent and separately testable — they share no code path — so this is one
plan with four self-contained tasks in no required order (though the tasks are
ordered registry-first so item 4's guard and item 3's field land before the
docs).

## Guiding invariants (unchanged by this work)

- **Engine purity.** The maintainer checker and docs live OUTSIDE
`plugins/hull/hull/`; nothing here adds `os`/`subprocess`/networking under the
engine, so `test_purity.py` stays green with no allowlist change. Item 2 edits
`scripts/sync_action_pins.py`, which is already stdlib-only maintainer tooling
outside the plugins.
- **Byte-identical goldens.** No rendered output changes. Item 3 moves *where*
the trufflehog advisory string is authored (scaffold.py → the registry entry),
not the string itself or anything rendered into a workflow. The three hull
goldens (`security.yml`, `security-license.yml`, `security-trufflehog.yml`)
stay byte-for-byte identical; this is the acceptance check for items 3 and 4.
- **Round-trip contract (#33).** Item 3 touches no signal keys and no
`propose_config`/`load_config` path — only the scaffold advisory/blocker text
and a new registry field with a default. `SIGNAL_KEYS` is untouched.

## Item 1 — the trufflehog pin's manual bump path (docs only)

**Problem.** `.github/dependabot.yml` scans `github-actions` in `/`, which only
sees action refs that appear in *this repo's own* workflow files.
`trufflesecurity/trufflehog` appears in none of them — by design, since
shipyard's own `security.yml` deliberately stays on gitleaks (personal-account
repo, no license gate to dodge). So Dependabot will never propose a bump for it,
and the pin ages silently. It is the first pin the suite ships to consumers
without dogfooding it, which is exactly what breaks the automated mechanism.

**Decision.** Document it; do not build automation for a single pin. A scheduled
CI checker or a maintainer-run script was considered and rejected as more
machinery than one pin warrants — the honest fix is to make the manual path
discoverable, not to hide the limitation behind partial automation.

**Change.** In `README.md`:

1. Add a row to the action-pin source-of-truth table:

| Action | Pinned in |
|---|---|
| `trufflesecurity/trufflehog` | `plugins/hull/hull/scanners.py` |

(placed after the existing `gitleaks/gitleaks-action` row, since both are
hull scanners.)

2. Add a short note beneath the table, in the maintainer/Development section
near where `sync_action_pins.py` is described:

> Dependabot only bumps action refs that appear in a workflow committed to
> *this* repo. Every pin above except `trufflesecurity/trufflehog` appears in
> `.github/workflows/`, so Dependabot bumps it and `sync_action_pins.py`
> propagates it into the registry. trufflehog does not — shipyard's own
> `security.yml` uses gitleaks — so when a new trufflehog release ships, bump
> `action_ref`/`action_ref_version` in `plugins/hull/hull/scanners.py` by
> hand, then run `sync_action_pins.py` to regenerate the goldens.

**Test.** Documentation only; no automated test. Acceptance is the two README
edits present and accurate (the named file `scanners.py` genuinely holds the
pin; the named script genuinely regenerates goldens).

## Item 2 — `sync_action_pins.py` regenerates all hull goldens

**Problem.** `regenerate()` rewrites only `plugins/hull/tests/golden/security.yml`.
All three hull goldens embed the `actions/checkout` SHA, so the next checkout
bump rewrites the registry and `security.yml`, then fails
`test_license_plan_matches_golden_byte_for_byte` and
`test_trufflehog_matches_golden_byte_for_byte` with no script support. #27 added
a third stale-able golden without touching the script.

**Change.** In the embedded regen script inside `regenerate()`, replace the
single hull golden write with a filename→config mapping covering all three, and
write each:

```python
# hull imports already present in the script: Config as HC, build_plan as
# hplan, render as hrender. The existing single write is
# hrender(hplan(HC(name="security", scanner="gitleaks")))
# -- a directly-constructed Config passed straight to build_plan, NO
# load_config. The map below keeps that exact shape.
hull_goldens = {
"security.yml": HC(name="security", scanner="gitleaks"),
"security-license.yml": HC(name="security", scanner="gitleaks",
license_secret="GITLEAKS_LICENSE"),
"security-trufflehog.yml": HC(name="security", scanner="trufflehog"),
}
# A golden on disk with no mapping here would be silently left stale after a
# pin bump -- the exact bug #30 item 2 fixes. Fail loudly instead.
from pathlib import Path as _P
on_disk = {p.name for p in _P("plugins/hull/tests/golden").glob("*.yml")}
missing = on_disk - set(hull_goldens)
assert not missing, f"hull goldens with no regen mapping: {sorted(missing)}"
for fn, cfg in hull_goldens.items():
open("plugins/hull/tests/golden/" + fn, "w").write(hrender(hplan(cfg)))
```

The exact constructor arguments (`Config` field names for name/scanner/license
secret) must be taken verbatim from how the existing hull golden tests
construct configs — the plan task must read `plugins/hull/tests/` and the
current `sync_action_pins.py` hull write, and copy the real field names (the
license-secret argument name in particular: `license_secret=` above is the
expected snake_case field, but the plan confirms it against `hull/config.py`
and the `security-license.yml` golden test rather than assuming).

**Guard rationale.** Deriving the *target list* from the directory (per the
issue's suggestion) while keeping an explicit filename→config map is deliberate:
a pure `glob()` cannot know which config produces which golden, but a bare
hardcoded map silently skips a newly added golden. The `missing` assertion is
the reconciliation — the directory is the source of truth for *what must be
regenerated*, the map is the source of truth for *how*, and a gap between them
is a loud failure at regen time.

**Test.** `sync_action_pins.py` has no unit test today and this is maintainer
tooling; acceptance is running it end-to-end and confirming (a) all three hull
goldens are rewritten byte-identically when no pin changed (`git diff` empty),
and (b) the `missing` assertion fires if a stray `.yml` with no mapping is
dropped into the golden dir (verified once manually, not committed as a test).

## Item 3 — move two per-scanner facts into `ScannerSpec`

**Problem.** `scaffold.py` hardcodes `"trufflehog"` twice — as the named remedy
in the organization blocker, and as the gate for the BASE==HEAD advisory
(`if scanner == "trufflehog":`). Every *other* per-scanner fact (action pin,
permissions, license env, `with:` inputs) lives in `ScannerSpec`. Register a
third scanner and it gets no advisory mechanism and is never offered as a
remedy, with nothing failing to tell you.

**Change A — advisory field.** Add to `ScannerSpec`:

```python
#: A scanner-specific advisory (non-fatal caveat) surfaced at init, or None.
#: A fact about the tool, so it lives here beside the pin -- registering a
#: scanner with a quirk worth stating gets an advisory channel automatically,
#: where the old `if scanner == "trufflehog"` gate gave a third scanner none.
advisory: Optional[str] = None
```

Move the trufflehog BASE==HEAD advisory string verbatim from `scaffold.py` into
the trufflehog registry entry's `advisory=`. gitleaks leaves it unset. In
`check_preconditions`, replace the `if scanner == "trufflehog":` block with:

```python
if REGISTRY[scanner].advisory is not None:
advisories.append(REGISTRY[scanner].advisory)
```

**Change B — derived remedy.** The org blocker currently ends by naming the
`"trufflehog"` scanner as the license-free alternative. Derive it instead:

```python
license_free = sorted(
sid for sid, spec in REGISTRY.items() if spec.license_env is None
)
```

and phrase the remedy from `license_free` (today `["trufflehog"]`). The blocker
only fires for a scanner whose `license_env is not None`, and
`test_at_least_one_registered_scanner_needs_no_license` guarantees
`license_free` is non-empty, so the derived list is always non-empty when the
blocker fires. Exact wording is resolved in the plan; it must name the
license-free scanner(s) from the list rather than a literal, and read naturally
for the one-element case (the only case today).

**Tests.**
- Existing `security-trufflehog.yml` golden stays byte-identical (the advisory
is init-time text, never rendered — but this confirms nothing rendered moved).
- A new test asserting `check_preconditions` with a trufflehog scanner still
surfaces the BASE==HEAD advisory (behavior preserved through the refactor).
- A new test asserting the org blocker's remedy text contains the derived
license-free scanner name (`"trufflehog"`) and that the string `"trufflehog"`
does **not** appear as a literal in `scaffold.py` source outside a comment —
i.e. the fact now comes from the registry. (Shape: read the module source,
assert no literal `"trufflehog"` in executable lines; a targeted check, since
the whole point is that the name is no longer hardcoded there.)
- The advisory-field test: `REGISTRY["trufflehog"].advisory` is set and
`REGISTRY["gitleaks"].advisory is None`.

## Item 4 — copy `scan_with` like `env`

**Problem.** `plan._build_job` does `with_=spec.scan_with` (by reference) while
`_scan_env` does `dict(spec.env)` (copied), and
`test_build_plan_does_not_mutate_the_registry_spec_env` guards the env copy but
nothing guards `scan_with`. A caller mutating `plan.jobs[0].steps[1].with_`
would corrupt `REGISTRY["trufflehog"].scan_with` process-wide. Latent, not live
(no consumer mutates it; process is short-lived), but an asymmetry the existing
guard test makes read as an oversight.

**Change.** In `_build_job`:

```python
with_=dict(spec.scan_with) if spec.scan_with else None,
```

preserving the existing None-when-falsy behavior (gitleaks has
`scan_with=None`; the renderer omits a falsy `with:`).

**Test.** `test_build_plan_does_not_mutate_the_registry_spec_scan_with`,
mirroring the existing env guard: build a plan for the trufflehog scanner, mutate
the resulting step's `with_`, and assert `REGISTRY["trufflehog"].scan_with` is
unchanged. The trufflehog golden stays byte-identical (the copy is
value-identical to the reference).

## Acceptance for the whole branch

- Full suite green, count strictly greater than the pre-branch baseline (new
tests in items 3 and 4; item 2 adds no committed test; item 1 none).
- `git diff` against the three hull goldens is empty (no rendered change).
- `test_purity.py` green with no allowlist edit.
- The round-trip property test green (no signal-key change).
2 changes: 1 addition & 1 deletion plugins/hull/hull/plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def _build_job(scanner_id: str, license_secret=None) -> Job:
)
scan_step = scanners.Step(uses=spec.action_ref,
env=_scan_env(spec, license_secret),
with_=spec.scan_with,
with_=dict(spec.scan_with) if spec.scan_with else None,
uses_version=spec.action_ref_version)
return Job(
id=spec.id,
Expand Down
31 changes: 13 additions & 18 deletions plugins/hull/hull/scaffold.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ def check_preconditions(signals) -> Preconditions:
blockers: list[str] = []
advisories: list[str] = []

license_free = sorted(
sid for sid, spec in REGISTRY.items() if spec.license_env is None
)

# The whole point of this guard. Without it, hull:init commits a workflow
# that is red on its very first run, for a reason that has nothing to do
# with the repo's code and is not stated anywhere in the file it wrote.
Expand All @@ -255,8 +259,9 @@ def check_preconditions(signals) -> Preconditions:
f"organization) as an Actions secret, and set \"licenseSecret\" in "
f".hull.json to that secret's name (conventionally "
f"\"{license_env}\") so hull renders it into the scan step -- or "
f"re-run hull:init choosing the \"trufflehog\" scanner, which "
f"needs no license, no secret, and fewer token permissions."
f"re-run hull:init choosing a scanner that needs no license "
f"({', '.join(license_free)}), which needs no secret and fewer "
f"token permissions."
)

# Non-fatal, but it will look like a hull bug the first time someone sees
Expand All @@ -274,22 +279,12 @@ def check_preconditions(signals) -> Preconditions:
f"than as a finding."
)

# Scanner-specific and deliberately an advisory, not a blocker. Unlike
# the organization gate above -- which fails EVERY run, given those
# conditions -- this one is an edge case: the action explicitly handles a
# branch's first push by setting BASE to empty, and an ordinary push or
# pull request has distinct base and head. Stated anyway so a rare red
# run is diagnosed rather than mistaken for a hull bug.
if scanner == "trufflehog":
advisories.append(
"The trufflehog action exits 1 with \"BASE and HEAD commits are "
"the same\" when the range it is asked to scan is empty. hull's "
"triggers make that rare -- a branch's first push is handled by "
"the action itself, and an ordinary push or pull request has a "
"distinct base and head -- but if you do see that message, it is "
"the action declining to scan nothing, not a finding and not a "
"hull bug."
)
# Scanner-specific and deliberately an advisory, not a blocker: a fact
# about the tool, carried in the registry beside its pin (ScannerSpec.
# advisory) rather than gated here by name, so a newly registered scanner
# with a caveat gets this channel automatically.
if REGISTRY[scanner].advisory is not None:
advisories.append(REGISTRY[scanner].advisory)

return Preconditions(tuple(blockers), tuple(advisories))

Expand Down
16 changes: 16 additions & 0 deletions plugins/hull/hull/scanners.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ class ScannerSpec:
#: user-supplied `with:` value would be a genuinely new decision, not an
#: extension of this one.
scan_with: Optional[dict] = None
#: A scanner-specific non-fatal advisory surfaced at hull:init, or None.
#: A fact about the tool, so it lives here beside the pin rather than as a
#: `scanner == "..."` gate in scaffold.py -- registering a scanner with a
#: quirk worth stating gets an advisory channel automatically, where the
#: old name-gate gave a third scanner none. A registry constant, never
#: user input; surfaced by scaffold.check_preconditions.
advisory: Optional[str] = None


REGISTRY: dict[str, ScannerSpec] = {
Expand Down Expand Up @@ -118,6 +125,15 @@ class ScannerSpec:
# which is the failure mode the organization blocker exists to
# prevent in the first place.
scan_with={"extra_args": "--results=verified,unknown"},
advisory=(
"The trufflehog action exits 1 with \"BASE and HEAD commits are "
"the same\" when the range it is asked to scan is empty. hull's "
"triggers make that rare -- a branch's first push is handled by "
"the action itself, and an ordinary push or pull request has a "
"distinct base and head -- but if you do see that message, it is "
"the action declining to scan nothing, not a finding and not a "
"hull bug."
),
),
}

Expand Down
12 changes: 12 additions & 0 deletions plugins/hull/tests/test_plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,15 @@ def test_scan_step_carries_the_specs_scan_with(monkeypatch):
def test_scan_step_has_no_with_when_the_spec_declares_none():
job = build_plan(Config(name="security", scanner="gitleaks")).jobs[0]
assert job.steps[1].with_ is None


def test_build_plan_does_not_mutate_the_registry_spec_scan_with():
"""The plan copies the registry's scan_with before handing it to the step,
exactly as it copies env -- mutating the shared spec would corrupt the
registry entry for every later plan built in the same process. trufflehog
is the scanner that actually declares a scan_with."""
before = dict(REGISTRY["trufflehog"].scan_with)
plan = build_plan(Config(name="security", scanner="trufflehog"))
# Mutating the rendered step's with_ must not reach back into the registry.
plan.jobs[0].steps[1].with_["extra_args"] = "MUTATED"
assert REGISTRY["trufflehog"].scan_with == before
Loading