Skip to content

fix: protect CLI sources and recipes, keep explicit reloads, report secret scans - #274

Merged
johnny4young merged 5 commits into
mainfrom
fix/post-review-2026-10
Oct 14, 2026
Merged

johnny4young merged 5 commits into
mainfrom
fix/post-review-2026-10

Conversation

@johnny4young

@johnny4young johnny4young commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Follow-up review of everything merged since 2026-10-06 (#251–#271).

Fixes

  • CLI could overwrite its source through a symlink (regression from fix(cli): reserve distinct canonical artifact destinations #268). NSString.pathComponents reports a link target ending in / (../out/, tab-completed paths) with a final "/" component, which canonicalPath treated as the root. Later components were then keyed from /, so render dir/notes.md --out link/notes.md with link -> dir/ passed the input guard and replaced notes.md with a PNG. Batch collisions through such aliases also went unnoticed.
  • --recipe was not protected. render a.swift --recipe team.json --out team.json wrote a PNG over the recipe, and a batch rendered a recipe that sat inside its input folder. The recipe now joins the background image and watermark logo as a protected resource, and batch discovery excludes it.
  • A live-file poll could cancel an explicit Reload. When the 650 ms poll fired during a reload, it cancelled it, and the click was silently lost (local edits were kept after “Reload from File”). The poll now waits on the in-flight reload instead.
  • The secret-scan summary never appeared after a scan that changed whether redactions existed, because an onChange(isEmpty) cleared it right away. The summary now stays visible while the redactions it describes still apply.
  • The editor's Format and Destination tooltips used unlocalized domain copy. They now use the localized summaries that Settings already uses.
  • A batch rendered into its own input folder could not run again. The earlier image was claimed as an input (output conflict), and earlier sidecars were rendered as new cards under renamed outputs. A non-text file exactly where the batch writes an image is now an earlier output, and so are the sidecars beside it. Markdown and HTML sidecars must also reference that image. Anything ambiguous stays a source and keeps failing closed.
  • Manual redactions survive a secret scan, and the scan-summary rule has unit tests (follow-up commits on this branch).
  • The --copy description no longer says that a PDF --out copies “the same image”. A PDF gets its own PNG render.
  • CI: after the history rewrite, the push before SHA no longer existed, so git rev-parse failed coverage setup on main (bd4a4cb). When before is not an ancestor of the pushed commit, the job now diffs against the parent. Pull requests still use their base SHA, fail-closed.

Verification

  • Every fix has a regression test, and each one fails without its fix: CLIOutputContractTests (trailing-slash links, relative and absolute), CLIInputSafetyTests (source preserved end to end), CLIRecipeTests (render and batch), CLIBatchArtifactTests (in-place rerun with and without sidecars, --no-overwrite, a real Markdown source beside an unrelated image), LivingSnapshotSessionTests.aPollCannotCancelAnExplicitReload.
  • make lint, make test (full suite) and cd site && npm test all pass locally.

Review follow-up (5c8755b)

  • SecretScanReport holds the scan summary rule, with a unit test that fails if the summary is cleared while its redactions still apply.
  • The editor and Settings share one destination tooltip (ExportPreset.localizedHelp(for:)).
  • Removed fileReadIsReload resets that only ran once fileReadTask was already nil; aPollCannotCancelAnExplicitReload still fails without the guard.
  • CI prints a notice when diff coverage falls back to the last commit (the gate is unchanged).
  • Local gates: make lint, make test (full suite). Hosted run: 38484840076.

Review follow-up (5add230)

  • The editor's secret scan adds the detected lines to the existing redactions instead of replacing them, matching --redact-secrets. aScanKeepsRedactionsTheUserAddedByHand fails without the merge.
  • Local gates: make lint, make test (full suite). Hosted run 38493433939 on 7836e83: the first Sequoia attempt failed only uploading performance evidence (ENOTFOUND); every test had passed, and the rerun of that job passed.

…ecret scans

- Ignore the trailing "/" that NSString.pathComponents reports for a symlink target
  ending in a slash; it reset canonical paths to the root, so
  `--out link/notes.md` with `link -> dir/` replaced the source with a PNG.
- Claim the --recipe file as a protected resource for outputs and batch reports,
  and exclude it from batch discovery.
- A live-file poll observes an in-flight explicit reload instead of cancelling it.
- Show the secret-scan summary while its redactions still apply; localize the
  inspector's Format and Destination tooltips.
- Correct the --copy description for PDF outputs.
- CI: fall back to the parent commit when a force push leaves `before` outside
  the pushed history, instead of failing coverage setup.
- Extract SecretScanReport so the summary visibility rule is unit-tested.
- Share the destination tooltip copy between the editor and Settings.
- Drop fileReadIsReload resets that only run once fileReadTask is nil.
- Note in CI when diff coverage falls back to the last commit.
- Correct the recipePath doc: batch discovery also excludes the recipe.
The editor's scan replaced the redacted lines with its own result, so a line
redacted by hand that the scanner does not detect was exported in plain text.
Merge the detected lines into the existing redactions, as --redact-secrets
already does.
A batch rendered into its input folder treated its earlier outputs as sources on
the next run: the earlier image was claimed as an input and the run failed with an
output conflict, and earlier sidecars were rendered as new cards under renamed
outputs. An earlier image is now a non-text file exactly where the run writes an
image, and an earlier sidecar sits beside one where the run writes that sidecar
(Markdown and HTML must also reference the image). Anything ambiguous stays a
source and keeps failing closed.
@johnny4young
johnny4young enabled auto-merge (squash) October 14, 2026 01:08
@johnny4young
johnny4young merged commit c1b400e into main Oct 14, 2026
10 of 11 checks passed
@johnny4young
johnny4young deleted the fix/post-review-2026-10 branch October 14, 2026 02:31
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.

1 participant