ci(soak): validate dispatch inputs, time out each iteration, compile once - #63
Merged
Conversation
…once
Two findings from the external review, both in the workflow written during this
session.
Input validation (soak-windows.yml:86)
--------------------------------------
`((i <= ITERATIONS))` is a code-execution sink: bash evaluates array subscripts
inside arithmetic, so ITERATIONS='a[$(cmd)]' runs the substitution.
Demonstrated rather than asserted — the payload wrote its marker file.
Passing inputs through `env:` instead of interpolating ${{ }} into the script
already closed the YAML-level injection; this closes the arithmetic one, which
is the half that remained. Dispatch needs write access, so it is hardening
rather than a hole a stranger can reach, but a workflow in a security repo
should not evaluate its own inputs.
iterations, gomaxprocs, mode and the new timeout are now checked before use, and
the same payload is rejected without being evaluated.
Diagnostics (soak-windows.yml:87)
---------------------------------
No per-iteration timeout meant a hang produced NOTHING: the job sat until its
own 90-minute limit and was killed with no traceback. After a crash, a deadlock
is the second most interesting outcome, and it was the one this workflow could
not report. Each iteration now carries -timeout / -test.timeout, so a hang
panics and dumps every goroutine — which with GOTRACEBACK=system is the evidence
wanted.
Compilation moved out of the loop. A compile error inside it failed all N
iterations identically and reported them as N crashes: a wall of false positives
burying the rare real one, and a "200 failures" line meaning the opposite of
what it appeared to. It now fails fast, before looping.
Failures are classified — runtime-crash / data-race / hang / test-failure — so
the number this workflow exists to produce is a rate for ONE phenomenon rather
than a total that silently mixes in flaky assertions. Patterns were checked
against real output: both recorded sightings, a real `go test -timeout` panic,
and an ordinary t.Fatal. A race report gets its own bucket because -race finding
a genuine write race would be the best lead available on a bug whose entire
signature is a stray write.
deadpoets
added a commit
that referenced
this pull request
Aug 19, 2026
…rting The comment added in #63 contains a literal empty `${{ }}`. Actions expands expressions in `run:` before any shell sees them, so the `#` protects nothing and the file has been unparseable since f6218b4 — zero-job runs against pushes, nothing on any PR's checks, so #64/#65/#66 all looked green.
deadpoets
added a commit
that referenced
this pull request
Aug 19, 2026
## The soak has not run since #63 The comment I added in #63 contains a literal empty `${{ }}`. Actions expands expressions in `run:` before any shell sees them, so the `#` protects nothing — the file has been unparseable ever since. ``` last success 2026-08-19T00:27:43Z workflow_dispatch 704806f first failure 2026-08-19T01:54:52Z push f6218b4 (#63) ``` It fails invisibly: a zero-job run against a **push**, never against the PR. #64, #65 and #66 each reported 22/22 green while the soak was dead. ## Commits 1. **`8b18b5a`** — drop the delimiter. Verified: dispatched run [32210902399](https://github.com/deadpoets/secmem/actions/runs/32210902399) completed successfully, 20 iterations. 2. **`390695d`** — `actionlint` in the `lint` job, pinned to v1.7.12. Verified to exit 1 on the pre-fix file and 0 on the fixed one. 3. **`ca25dbc`** — fingerprint the runner on every soak run. ## Fingerprint, first result | | workstation | `windows-latest` | |---|---|---| | OS | Win 11 Insider 26220 (client) | **Server 2025** Datacenter 26100 | | CPU | Intel Core Ultra 7 265KF, 20 logical | **AMD EPYC 7763**, 4 logical | | Mandatory ASLR | **ON** | off | | HVCI | **running** | not running | | Defender realtime | **on** | off | | Scheduler quantum | 2 | 2 — *same* | Two differences I expected turned out not to exist: both hosts are build 261xx, and both run the client quantum. Worth having measured rather than assumed. Separately: **`GOMAXPROCS=4` does not emulate a 4-CPU machine.** It caps Go's Ps while the OS still spreads sysmon, GC workers and the race detector's threads across all cores — so the ~1190 local samples were never the runner's contention profile. --------- Co-authored-by: Chris Fink <7587613+deadpoets@users.noreply.github.com>
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
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.
The two INFO findings in the soak workflow — both in code written during this
session, both worth fixing on their own merits.
1. Dispatch inputs reached a bash arithmetic context —
:86((i <= ITERATIONS))is a code-execution sink. Bash evaluates array subscriptsinside arithmetic, so
ITERATIONS='a[$(cmd)]'runs the substitution.Demonstrated, not asserted:
Passing the inputs through
env:rather than interpolating${{ }}into thescript already closed the YAML-level injection. This closes the arithmetic one,
which is the half that remained.
Dispatching needs write access, so this is hardening rather than a hole a
stranger can reach — but a workflow in a security repo should not evaluate its
own inputs.
iterations,gomaxprocs,modeand the newtimeoutare validated beforeuse:
2. A hang produced zero evidence; a compile error produced N false ones —
:87No per-iteration timeout. A hang meant the job sat until its own 90-minute
limit and was killed with no traceback. After a crash, a deadlock is the second
most interesting outcome — and it was precisely the one this workflow could not
report. Each iteration now carries
-timeout/-test.timeout, so a hang panicsand dumps every goroutine, which with
GOTRACEBACK=systemis exactly theevidence wanted.
Compilation was inside the loop. A compile error failed all N iterations
identically and reported them as N crashes — a wall of false positives burying
the rare real one, and a
failures: 200line meaning the opposite of what itappeared to. It now compiles once and fails fast.
Failure classification
The number this workflow exists to produce is a rate for one phenomenon, so
failures are now bucketed
runtime-crash/data-race/hang/test-failurerather than totalled together.Patterns were checked against real output rather than guessed:
fatal error: stack not a power of 2(sighting 1)runtime-crashfatal error: acquireSudog: found s.elem != nil(sighting 2)runtime-crashgo test -timeoutpanichangt.Fataltest-failureWARNING: DATA RACEdata-raceA race report gets its own bucket because
-racefinding a genuine write racewould be the best lead available on a bug whose entire signature is a stray
write — it must not be filed beside a flaky assertion.
Validated: bash syntax over the extracted 148-line script, YAML parsed, and the
injection/validation behaviour exercised directly.