Skip to content

build(sfw): bump to 1.14.0, retire the SFW_SHIM_ACTIVE_CARGO escape hatch - #577

Open
jdalton wants to merge 1 commit into
nubjs:mainfrom
jdalton:build/sfw-1.14-drop-cargo-sentinel
Open

build(sfw): bump to 1.14.0, retire the SFW_SHIM_ACTIVE_CARGO escape hatch#577
jdalton wants to merge 1 commit into
nubjs:mainfrom
jdalton:build/sfw-1.14-drop-cargo-sentinel

Conversation

@jdalton

@jdalton jdalton commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

CI has been running cargo outside Socket Firewall on purpose. A permanent escape hatch, SFW_SHIM_ACTIVE_CARGO: "1", told the shim to let cargo through unscanned — so every crate the build downloaded was a blind spot in the one place that is supposed to have none. This PR deletes that escape hatch, because Socket Firewall 1.14.0 fixes the thing that forced it.

It also bumps zizmor, pnpm, and npm to their newest releases that have cleared the soak window, and adds automation so the soak paperwork cleans itself up instead of turning CI red on a day nobody changed any code.

Why the cargo bypass existed — the firewall was proxying nub's own localhost test mocks

The firewall proxy used to intercept everything, and nub's test binaries talk to registry mocks running on localhost. Proxying that traffic mangled it — ECONNRESET, and 405s on registry writes — so the only way to keep the test suite green was to take cargo out of the firewall entirely.

sfw 1.14.0 exempts loopback addresses in the child's NO_PROXY (localhost,127.0.0.1,::1). Local mocks are never proxied now, so cargo can ride the firewall like every other package manager, and the bypass has nothing left to justify it.

What is in sfw 1.14.0 — two fixes, and the soak paperwork for adopting it early

Two fixes matter to us:

Fix Why we want it
Loopback NO_PROXY exemption local test mocks stop being proxied, which is what retires the cargo bypass
Diagnostics moved to stderr a command like pnpm --version run through a shim keeps clean, parseable stdout

sfw 1.14.0 was published on 2026-07-23, which is inside our 7-day soak window. Adopting a release early requires a dated soakBypass annotation on the pin, and this PR carries those annotations (they clear on 2026-07-30). After that date the checks will warn that the annotations are stale, and the new autofix workflow (below) prunes them automatically. If this PR is still open then, one local pnpm run tools:fix refreshes it.

Why expired soak annotations now warn instead of failing — and the daily job that prunes them

An expired bypass annotation means the version has fully soaked. The bypass no longer bypasses anything, and the pin is still checksum-verified — so failing CI over it turned harmless stale paperwork into a red gate that flips overnight with zero code change.

So the gate now splits by cause:

Annotation state Result
Expired but valid warns, exit 0 — the version soaked, nothing is being skipped
Missing, malformed, or wrong arithmetic fails hard — an unauditable bypass is a real problem

The cleanup is automated. A daily soak-autofix workflow runs the fixers and, when anything changed, commits to a bot branch and opens a PR. It only runs from main (a manual dispatch on a topic branch cannot force-push the bot branch), serializes via a concurrency group, and captures the fixers' exit status so a crashed fixer fails the run after the mechanical cleanup is committed.

One thing to know: PRs opened with the default github.token do not trigger CI. Setting an optional SOAK_AUTOFIX_TOKEN secret (a fine-grained PAT) makes the bot PRs run checks like any other.

The other pinned-tool bumps — three versions, plus musl, shim, and gate hardening

Each version below is the newest release that has already cleared the soak window, so none of them needs a bypass annotation.

Tool Bump
zizmor 1.26.1 → 1.28.0
pnpm 11.8.0 → 11.15.1
npm 12.0.0 → 12.0.1

Still soaking and deliberately left alone: pnpm 11.16/11.17, taze 19.16.0, and SkillSpector's 2.4/2.5 line.

Three smaller hardening fixes ride along:

Area What it fixes
musl support the platform detector now recognizes musl hosts via the loader heuristic, so the -musl pnpm pins actually get used — previously a musl host silently installed a glibc binary
Shims fail-open now warns once on stderr instead of being silent; SFW_UNKNOWN_HOST_ACTION=ignore is exported so the enterprise tier cannot block non-registry hosts the day a SOCKET_SECURITY_KEY secret lands; rustup detection honors CARGO_HOME
Gate robustness the exclude-list parser now tolerates a trailing comment on the key line — previously that silently skipped validating everything beneath it — and a malformed package.json produces a finding instead of a crash

Ran: all SRIs were recomputed from the release assets; tools:check, soak, and test:scripts (64 tests) are green.

@vercel

vercel Bot commented Jul 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nub Ready Ready Preview Jul 30, 2026 6:19pm

Request Review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — a tooling/CI build PR that bumps Socket Firewall to 1.14.0 and drops the SFW_SHIM_ACTIVE_CARGO CI escape hatch that its new loopback exemption makes redundant.

  • Retire the cargo escape hatch — removes SFW_SHIM_ACTIVE_CARGO: "1" from the test job env in .github/workflows/ci.yml, so cargo now routes through the sfw shim like the other package managers. Verified against primary sources: sfw 1.14.0 (both SocketDev/sfw-free and SocketDev/firewall-release releases, 23 Jul) genuinely adds "exempt loopback in NO_PROXY so localhost bypasses the proxy" (#133) and "send sfw's own diagnostics to stderr", so loopback registry-mock traffic is no longer proxy-mangled.
  • Bump sfw 1.13.1 → 1.14.0sfw-free and sfw-enterprise in external-tools.json, each with a soakBypass block. The window arithmetic is valid (SOAK_DAYS = 7; published 2026-07-23 + 7 = 2026-07-30 = removable), and checkPins reads soakBypass regardless of key position.
  • Soaked bumps — zizmor 1.26.1 → 1.28.0, pnpm 11.8.0 → 11.15.1, npm 12.0.0 → 12.0.1, all past the window with no bypass; every SRI recomputed. Per-asset hashes aren't independently re-downloaded here, but the install-time download() SRI-verifies each one, so a wrong hash self-gates the build.

The SFW_SHIM_ACTIVE_CARGO removal leaves no stale references — the shim's own SFW_SHIM_ACTIVE_${cmd} recursion sentinel in scripts/soak/external-tools.mts is a separate mechanism and is unaffected.

ℹ️ Nitpicks

  • Whatever re-serialized external-tools.json flipped every prose em-dash from the literal to a \u2014 escape (8 lines). This is cosmetic — the JSON still parses and package.json already uses the escaped form — but it also touched the skillspector description (line 174) whose version is otherwise unchanged in this PR, adding diff noise to an unrelated entry. Not blocking; consider keeping the literal char to minimize the diff.

Pullfrog  | View workflow run | Using Claude Opus𝕏

@jdalton

jdalton commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Added the fail-closed→auto-heal half: tools:fix prunes expired soakBypass annotations, and a daily soak-autofix workflow commits the pruning as a bot PR — so the 2026-07-30 expiry of this PR's sfw annotations resolves itself post-merge instead of turning the soak gate red. (If this PR is still open on the 30th, one local pnpm run tools:fix refreshes it.)

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — minor hardening suggestions on the new autofix workflow, inline and below.

Reviewed changes — the new commit adds an auto-prune path for expired soakBypass annotations plus a scheduled workflow to land the pruning, so the fail-closed soak gate self-heals instead of sitting red waiting for a human.

  • Add pruneExpiredSoakBypasses + wire it into --fix — deletes soakBypass blocks whose removable date is in the past, skipping missing and malformed (non-ISO) annotations so those stay findings for a human. main --fix prunes, writes the JSON back, then re-loads and runs checkPins. Logic verified sound: the re-read picks up the pruned state, and isValidIsoDate correctly rejects 2026-13-45.
  • Add .github/workflows/soak-autofix.yml — daily cron plus workflow_dispatch; runs both fixers, commits any changes to bot/soak-autofix (force-push), and opens a PR when none is open on that head. Optional SOAK_AUTOFIX_TOKEN PAT so the bot PR actually triggers CI.
  • Add the pruneExpiredSoakBypasses unit test — covers fresh/expired/malformed/unannotated pins and idempotency. Good, focused coverage.
  • Document tools:fix + the autofix workflow — new tools:fix script in package.json and a note in the soak skill.

ℹ️ Autofix commits and opens a PR on git diff --quiet alone, with no post-fix re-check

The workflow runs both fixers with || true, then decides whether to commit purely on whether the working tree is dirty. It never re-runs --check (or pnpm run soak) to confirm the gate is actually green before committing and opening the PR. Because soak.mts --fix writes ~5 tracked files sequentially in a loop with no transaction, a fixer that crashes partway leaves a partial state that is still "dirty" — so it gets committed and PR'd with no signal that the result doesn't clear the gate. This is a rough edge rather than a merge blocker: a human reviews the resulting PR, and with SOAK_AUTOFIX_TOKEN set, CI runs on it too.

Technical details
# Autofix commits without re-validating the gate

## Affected sites
- `.github/workflows/soak-autofix.yml:44-46` — both fixers run with `|| true`, so a crash is swallowed the same as "intentional human-only findings remain".
- `.github/workflows/soak-autofix.yml:51-71` — commit/push/PR is gated on `git diff --quiet` only; no re-check that the fixed state passes.
- `scripts/soak/soak.mts:401-423``--fix` writes `cargoConfig`, `npmrc`, `workspaceYaml`, `renovateJson` in a sequential loop; a mid-loop crash yields individually-valid but collectively-partial state.

## Required outcome
- The workflow should not open a PR claiming the windows are pruned when the gate would still be red (or the fix is partial).

## Suggested approach (optional)
- After the fixers, run the checks (`node scripts/soak/soak.mts --check` + `node scripts/soak/external-tools.mts --check`) and either fail the job or annotate the PR body when problems remain — so a partial/failed fix is visibly distinguished from a clean one, rather than relying on `git diff --quiet`.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread .github/workflows/soak-autofix.yml

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — one minor doc nit inline.

Reviewed changes — the new commit reframes an expired-but-valid soak bypass as a warning (exit 0) rather than a hard failure, so a date boundary no longer reddens CI overnight with zero code change.

  • Warn instead of fail on cleared bypass windows — removes the expiry-failure branch from checkPins (external-tools.mts) and checkExcludeAnnotations (soak.mts); a version whose removable date has passed has already soaked, so dropping the bypass can't change the install decision. Verified sound.
  • Add staleBypasses / staleExcludes — each returns the names of valid-but-expired entries; main() console.warns them without touching the exit code. Missing, malformed, and wrong-arithmetic annotations still hard-fail their respective checks (unauditable, so kept failing).
  • --fix convergence intactpruneExpiredSoakBypasses and fixWorkspaceYaml still prune the stale entries, so the daily soak-autofix workflow drives the ledger back to clean.
  • Tests updated — both suites now assert the checks return [] on expired-but-valid input while the stale lists return the name. Focused coverage.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread .claude/skills/soak/SKILL.md Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — the new hardening code is sound; one doc-cleanup gap left over from the warn-not-fail reframe.

Reviewed changes — the new commit (83096cb, "address review-bot findings") adopts the two prior hardening threads on the autofix workflow and adds several defensive fixes to the soak scripts.

  • Harden the autofix workflow — the autofix job now carries if: github.ref == 'refs/heads/main' and a concurrency: soak-autofix group (cancel-in-progress: false), and fixer exit status is captured and re-raised after the mechanical commit instead of || true, so a crashed fixer or human-only finding can't ride a green run.
  • musl-aware platformKeyexternal-tools.mts detects musl via loader presence and appends -musl, so a musl host no longer silently resolves the glibc pin; tools with no -musl pin now throw no pinned asset loudly.
  • CARGO_HOME-aware RUSTUP_CARGOpaths.mts resolves the rustup cargo shim under $CARGO_HOME/bin instead of hardcoding ~/.cargo.
  • Fail-open-not-silent sfw shim — the generated shim now emits [sfw-shim] sfw not on PATH — running <cmd> unfirewalled on stderr when the firewall is missing, skipping the sentinel re-entry path.
  • Comment-tolerant exclude parser + malformed-JSON handlingparseExcludeEntries opens the block even with a trailing comment on the key line (closing a silent-escape blind spot), and checkCatalogParity reports a broken package.json as a finding instead of crashing. Both covered by new focused tests.

ℹ️ The warn-not-fail reframe's stale "fail closed on cleared window" premise survives in three doc/comment sites

Commit c193fc3 changed a cleared bypass window from a hard failure to a warning (exit 0), and this commit updated the soak:fix paragraph in the skill accordingly. But three sibling notes still describe the old fail-closed-on-clear behavior, so the docs now contradict the code and each other. This is the same concern as the still-open prior thread on SKILL.md:34; the fix was partial.

Technical details
# Doc/comment notes still say a cleared window fails closed

## Affected sites
- `.claude/skills/soak/SKILL.md:34` — "The gates fail closed when a bypass window clears" — false as of `c193fc3`; a cleared window now warns (exit 0). (Already tracked in open thread `PRRT_kwDOSwTR0s6UTNAG`.)
- `.github/workflows/soak-autofix.yml:3-4` — "an expired soakBypass annotation or a cleared minimumReleaseAgeExclude pin turns tools:check / soak red until someone prunes it" — same stale premise.
- `tools/pnpm-workspace.yaml:20-21` — "`pnpm run soak` rejects unannotated or expired pins" — expired pins now warn, not reject. This hunk was edited by this commit (the schematic-example lines just below), so the inaccuracy sits inside a touched block.

## Required outcome
- All three notes describe the current model: a *malformed / missing / wrong-arithmetic* annotation fails closed (red), but a *cleared / expired* window only warns and is pruned by `--fix` / the daily workflow.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread tools/pnpm-workspace.yaml Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — one informational note on the new shim comment's free-tier claim.

Reviewed changes — the new commit exports SFW_UNKNOWN_HOST_ACTION=ignore in the generated sfw shim so that once a Socket key upgrades the firewall to the enterprise tier, its default block-on-unknown-hosts posture doesn't break the CI test job's traffic to non-registry hosts.

  • Set SFW_UNKNOWN_HOST_ACTION=ignore before exec sfw — added to writeShims in scripts/soak/external-tools.mts, on the sfw-present exec path only (after the recursion sentinel, never on the fail-open passthrough branch). Verified against Socket's docs: the variable is real and documented for enterprise wrapper mode, where unknown hosts default to block — so the enterprise half of the justifying comment is accurate, and the change is correctly motivated by the SOCKET_SECURITY_KEY secret the test job wires up.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread scripts/soak/external-tools.mts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — one minor test-coverage note inline.

Reviewed changes — the new commit (2ad6270, "take review fixes surfaced on the aube twin") carries four small hardening fixes ported from a parallel review on the aube twin PR; no product code.

  • Lease-checked bot-branch pushsoak-autofix.yml replaces the unconditional git push -f with git fetch origin "$BRANCH" then git push --force-with-lease, so a concurrent actor's commits on the shared bot/soak-autofix branch are never clobbered. The first-run case (no remote branch yet) is handled by the 2>/dev/null || true on the fetch, and the bare lease succeeds when the remote ref is absent.
  • Multi-toolchain-aware Docker prebake checkcheckDockerPrebake now parses the full argument list of each rustup toolchain install line instead of substring-matching, so toolchain install 1.91.0 1.93.0 satisfies an msrv of 1.93. The startsWith(${rustVersion}.) guard avoids a 1.91.93.0 false prefix match.
  • Windows cargo.exe pathpaths.mts resolves RUSTUP_CARGO to cargo.exe on win32.
  • Userinfo-tolerant remote normalizationnormalizeGitHubRepo adds an optional user@ group after https://, so a token-embedded HTTPS remote (e.g. https://x-access-token:TOKEN@github.com/...) is recognized instead of read as drift.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread scripts/soak/external-tools.mts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — the warn-not-fail prose alignment is correct; one sibling comment was left stale.

Reviewed changes — the new commit (af44d07, "docs(soak): align prose with warn-not-fail; source-cite the unknown-host comment") is a docs/comment-only pass that reconciles three stale notes with the warn-not-fail model from c193fc3 and rewrites the shim's unknown-host comment to a source-backed claim.

  • Reconcile three fail-closed-on-clear notesSKILL.md, tools/pnpm-workspace.yaml, and the shim comment now distinguish invalid states (missing / malformed / wrong-arithmetic → fail closed) from expired-but-valid windows (warn, pruned by soak:fix / the daily workflow). The three prior review threads on these sites are resolved.
  • Source-cite the unknown-host comment — the free-tier half is reworded from the unverifiable "hardcodes ignore and disregards the var" to "only the enterprise build reads the var — it is inert for the free tier," matching what Socket's docs actually state.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread .github/workflows/soak-autofix.yml Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues in this commit.

Reviewed changes — the new commit (d2f6dbb, "never prune a wrong-arithmetic annotation as 'cleared'") closes a consistency gap between the soak checkers and the stale/prune paths, so a too-early removable date can no longer be silently pruned while the checker still fails it.

  • Guard the stale/prune paths against wrong-arithmetic annotationsstaleBypasses and pruneExpiredSoakBypasses (external-tools.mts) plus staleExcludes and fixWorkspaceYaml (soak.mts) now require removable === published + SOAK_DAYS before applying the removable < today stale/prune test. Previously a wrong-arithmetic annotation with a past removable (e.g. published: today | removable: 2020-01-02) read as "cleared" and was pruned/warned, even though checkPins / checkExcludeAnnotations treat that exact shape as a hard failure — so the fixer could silently drop a bypass whose real window is still open. All four sites validate published via isValidIsoDate before addDaysIso, and fixWorkspaceYaml's === addDaysIso(...) comparison inherently rejects a malformed removable, so there is no un-validated arithmetic path.
  • Add wrong-arithmetic coverage — the pruneExpiredSoakBypasses test gains a wrongmath entry asserting it is neither pruned nor stale, and soak.test.mts adds a focused test that staleExcludes returns [], fixWorkspaceYaml keeps the entry, and checkExcludeAnnotations still flags it. Ran both suites locally — 36 pass, 1 skipped (the Dockerfile-present conditional).

Because main() re-runs s.check(...) on the post-fix body, a wrong-arithmetic entry the fixer correctly leaves in place still surfaces as an exit-1 finding under --fix — so the daily autofix workflow fails after the mechanical cleanup rather than papering over it. Consistent and correct.

Not approving only because prior Pullfrog threads remain open (the stale fail-closed-on-clear comment on .github/workflows/soak-autofix.yml:3-4, and its SKILL.md sibling). This commit doesn't touch those files, so those doc nits are left for the author — no action needed for the code in this commit.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — the download retry hardening is sound; one minor test-coverage note inline.

Reviewed changes — the new commit (5b16c0b, "downloads fall back to unauthenticated and retry once on 5xx") hardens download in scripts/soak/external-tools.mts so a token that a public cross-repo asset endpoint rejects, or a transient GitHub 5xx, no longer fails the install outright.

  • Refactor to a token-derived attempt(withAuth) closure — replaces the single headers-block fetch with a closure that issues a fresh fetch (and fresh AbortSignal.timeout(120s)) per attempt. On any !res.ok after an authed attempt it retries unauthenticated; on res.status >= 500 it sleeps 2s and retries unauthenticated once more. Verified sound — intermediate failed responses are discarded and the body is read only once on the final res, so there is no response-body reuse bug, and the always-unauthenticated retry matches the documented "public assets need no credential" intent.
  • Add the auth→no-auth fallback test — a 500 on the authed fetch followed by a successful unauthenticated retry, asserting the second attempt drops the authorization header.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread scripts/soak/external-tools.test.mts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — one minor consistency note inline, latent-only on the current config.

Reviewed changes — the new commit (a13e015, "stop the fixers reformatting files they do not own") rewrites the renovate-config fixer to a textual line rewrite and tightens the trailing-whitespace regex in two sibling fixers so a fix no longer deletes surrounding blank lines.

  • Rewrite fixRenovateConfig to a textual line edit — replaces the JSON.parse + re-stringify round-trip with a regex that rewrites only the minimumReleaseAge line, so hand-written single-line arrays and unrelated packageRules survive verbatim; handles the disabled, present, and absent-key cases, with an empty-object guard against producing {,\n...}. Covered by a focused "no reformatting churn" test.
  • Add an internalChecksFilter check to checkRenovateConfig — demands "strict" when the soak window is active, since renovate's default "flexible" internal-checks mode can raise an update that hasn't cleared minimumReleaseAge. The tracked .github/renovate.json already sets strict, so main --check stays green.
  • Tighten the trailing match in fixNpmrc / fixWorkspaceYaml\s*$[ \t]*$, so the replace no longer consumes the newlines that follow the key and silently collapses following blank lines.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread scripts/soak/soak.mts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found in these commits. Not approving only because prior Pullfrog threads remain open on files these commits don't touch.

Reviewed changes — the two new commits since the prior review (c143176) that harden the cargo leg of the dependency-update soak against a silent no-op.

  • Fail loudly when an old nightly ignores the cargo soakupdateCargo now captures cargo's stderr and treats the unused config key \unstable.min-publish-age`warning as a hard failure, so a nightly that doesn't implement the[unstable]` key can no longer update the lockfile with no soak window while exiting 0.
  • Exported, unit-tested detectorisMinPublishAgeUnsupported pins cargo's exact wording (escaped-dot regex) and is covered by a 4-case test: the real warning (true), a sibling unstable.other key, an empty string, and an ordinary progress line (all false).
  • SKILL.md note — documents that a merely-old nightly silently disables the window and that deps:update now detects and fails on it.

The root-cause verification (measured against nightly 2026-03-21) is the right fix rather than a guardrail, and it lands with focused coverage. stderr is faithfully re-emitted after capture; the only behavior change is that cargo's stderr progress is now buffered until the command completes instead of streaming live — inherent to needing to inspect stderr, output fully preserved, not worth changing. Ran node --test scripts/soak/update-deps.test.mts locally: 4 pass, 0 fail. No prior Pullfrog thread is anchored in these hunks, so none were retired.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

@jdalton

This comment was marked as outdated.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found. Not approving only because prior Pullfrog threads remain open on surfaces this commit doesn't fully resolve.

Reviewed changes — the new commit (570b6b6, "take the adversarial-review findings") reworks the autofix push to be force-free, splits download's retry by status meaning, and tightens the --fix prune guard; all tooling/CI, no product code.

  • Force-free autofix pushsoak-autofix.yml drops --force-with-lease (correctly called out as security theater: the wildcard-refspec fetch moves the tracking ref to whatever a concurrent actor just pushed, so the lease is always satisfied) for a stash → base-on-origin/bot/soak-autofixstash pop → plain git push flow. A human's commits on the open autofix PR are preserved by construction; a genuine non-fast-forward fails the push loudly instead of being resolved by deletion. Traced sound.
  • Retry semantics split by statusdownload now drops the token only on 401/403/404 (a public cross-repo endpoint rejecting an Actions token) and keeps auth on a >=500 retry. This fixes a real bug: the old >=500 path dropped auth and made a private asset (sfw-enterprise) 404 on retry. Covered by the new keeps-auth-across-5xx test and the reworded 404 fallback test.
  • --fix prune set now equals the warn setfixWorkspaceYaml gains VERSION_PIN_RE.test(entry.name) + the wrong-arithmetic guard, so a bare-name / @scope/* standing-trust entry sitting under an expired annotation line is no longer silently deleted by --fix (which would re-arm the soak for a whole scope).
  • Pin decmpfs exactvendor/aube/Cargo.toml moves "0.1""=0.1.0" (0.1.2's FICLONE ioctl request breaks the musl build), matched by a <=0.1.0 renovate packageRule. renovate.json is otherwise a mechanical single-line→multi-line array reformat.

The prior open threads on the multi-toolchain positive test (external-tools.mts:224), the 503 test's 2s sleep (external-tools.test.mts:233), and the stale fail-closed-on-clear doc/comment notes are not touched or only partially addressed by this commit — left to the author, no new action needed for the code here.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — prior threads all cleanly resolved; one minor test-coverage note inline.

Reviewed changes — the new commit (e8b00d2, "take the review findings — one is a regression I introduced") resolves the four open review threads and adds one new integrity guard to checkPins; all tooling/CI, no product code.

  • Guard against a stale soakBypass versioncheckPins now hard-fails when a bypass's version no longer equals the pin's version, so a bumped pin can't ride an annotation granted for the old release. Type-safe and passes the real config (both sfw pins carry a matching soakBypass.version).
  • Rewrite the soak-autofix.yml header — the last surviving "fail closed on cleared window" note now describes the warn-not-fail model and frames the workflow as convergence rather than rescue.
  • Fix the fixRenovateConfig trailing-whitespace regression\s*$[ \t]*$, matching the two sibling fixers, so the fixer no longer swallows a following blank line.
  • Add the two missing positive-path tests — a multi-toolchain rustup toolchain install 1.91.0 1.93.0 satisfying an msrv of 1.93, and a first-503-then-success download retry asserting exactly two attempts.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread scripts/soak/external-tools.mts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found. This closes the last outstanding review finding on the PR.

Reviewed changes — the new commit (acbf127, "cover the soakBypass version-mismatch guard") is test-only: it adds the one missing case for the integrity guard added in the previous commit.

  • Lock in the soakBypass version-mismatch finding — the checkPins soakBypass test now clones the passing fixture, sets soakBypass.version to 0.9.0 against a pin of 1.0.0, and asserts the finding matches /soakBypass is for/. The assertion is non-vacuous: the mismatch push in checkPins precedes the date checks and the preceding assert.deepEqual(checkPins(good), []) proves the fixture is otherwise clean, so [0] is the mismatch message and dropping the guard makes assert.match throw on undefined.

This is the fourth and final entry in the coverage series this PR worked through, and every prior Pullfrog thread is now resolved. Verified locally: node --test scripts/soak/external-tools.test.mts → 17 pass, 1 skipped (the Dockerfile-prebake conditional), 0 fail; soak.mts --check and external-tools.mts --check both exit 0.

One timing note for whoever merges, not a defect: both sfw pins carry removable: 2026-07-30, and staleBypasses only reports on removable < today, so today is the last day the annotations read as fresh. From tomorrow they warn at exit 0 and tools:fix (or the daily soak-autofix run) prunes them — exactly the convergence this PR built, so nothing goes red either way.

Pullfrog  | View workflow run | Using Claude Opus𝕏

…atch

Bumps Socket Firewall to 1.14.0 and removes the SFW_SHIM_ACTIVE_CARGO
escape hatch, routing the cargo shim through the normal firewall path.
Folds in the soak/pinned-tooling review and adversarial-review fixes made
on top, including the decmpfs=0.1.0 exact pin (0.1.2 breaks musl) and the
soakBypass version-mismatch guard with its test.
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