chore: Make the security-scan log path safe for uncontainerised jobs #33

New issue

Open

opened 2026-09-04 06:04:44 +00:00 by ahmad · 0 comments

ahmad commented

2026-09-04 06:04:44 +00:00

Owner

Context

security-scan writes to a fixed /tmp/security-scan.log and truncates it when the action starts, on the premise that a runner may reuse /tmp between jobs. Raised in the review of #32 as a question rather than a finding: if the runner shares a filesystem across concurrently-running jobs, two security jobs would interleave into one file and each would see a mixed tail.

Not reachable today, and here is the evidence rather than the assumption:

ahmad/expiro     security job: containerised (container: node:22-bookworm)
ahmad/portfolio  security job: none

The only consumer runs the action inside a per-job container, so /tmp is that container's. The exposure begins the moment a consumer adds a security job without a container:.

The consequence is a mixed or truncated tail on a failure — diagnostic quality, not a wrong exit code. That is why it was not a blocker for #32.

Why it was not simply fixed there

RUNNER_TEMP and runner.temp are referenced nowhere in this repository or the consuming ones, so their behaviour on this runner is untested. If runner.temp expands to empty, outputs.log-path becomes /security-scan.log while the shell writes somewhere else — a silently wrong path, which is a worse failure than the one it prevents. Swapping a known-good constant for an unverified expression, on an approved change, to fix something no consumer can currently hit, is the wrong order to do things in.

Scope

In: determine whether this runner isolates /tmp per job; if it does not, make the log path unique per job and keep outputs.log-path agreeing with it.
Out: changing what the scanners do, or the truncate-on-start behaviour, which is correct either way.

Acceptance criteria

Given the runner's filesystem model, when it is checked, then the answer is recorded on this issue — /tmp isolated per job, or shared.
Given ${{ runner.temp }} is used in a workflow on this runner, when the job runs, then it is confirmed to expand to a non-empty per-job path before anything depends on it.
Given the path is made unique, when a consumer reads outputs.log-path, then it names the same file the action wrote — verified by a consuming job, not by inspection.
Given two security jobs run concurrently without containers, when both fail, then each posts only its own output.

Notes

The second criterion is the one that matters. The reason this is an issue rather than a commit is precisely that the obvious fix depends on an untested assumption, and substituting one unverified premise for another is not progress.

### Context `security-scan` writes to a fixed `/tmp/security-scan.log` and truncates it when the action starts, on the premise that a runner may reuse `/tmp` between jobs. Raised in the review of #32 as a question rather than a finding: if the runner shares a filesystem across concurrently-running jobs, two `security` jobs would interleave into one file and each would see a mixed tail. **Not reachable today, and here is the evidence rather than the assumption:** ``` ahmad/expiro security job: containerised (container: node:22-bookworm) ahmad/portfolio security job: none ``` The only consumer runs the action inside a per-job container, so `/tmp` is that container's. The exposure begins the moment a consumer adds a `security` job **without** a `container:`. The consequence is a mixed or truncated tail on a failure — diagnostic quality, not a wrong exit code. That is why it was not a blocker for #32. ### Why it was not simply fixed there `RUNNER_TEMP` and `runner.temp` are referenced nowhere in this repository or the consuming ones, so their behaviour on this runner is untested. If `runner.temp` expands to empty, `outputs.log-path` becomes `/security-scan.log` while the shell writes somewhere else — a silently wrong path, which is a worse failure than the one it prevents. Swapping a known-good constant for an unverified expression, on an approved change, to fix something no consumer can currently hit, is the wrong order to do things in. ### Scope - In: determine whether this runner isolates `/tmp` per job; if it does not, make the log path unique per job and keep `outputs.log-path` agreeing with it. - Out: changing what the scanners do, or the truncate-on-start behaviour, which is correct either way. ### Acceptance criteria - [ ] Given the runner's filesystem model, when it is checked, then the answer is recorded on this issue — `/tmp` isolated per job, or shared. - [ ] Given `${{ runner.temp }}` is used in a workflow on this runner, when the job runs, then it is confirmed to expand to a non-empty per-job path **before** anything depends on it. - [ ] Given the path is made unique, when a consumer reads `outputs.log-path`, then it names the same file the action wrote — verified by a consuming job, not by inspection. - [ ] Given two `security` jobs run concurrently without containers, when both fail, then each posts only its own output. ### Notes The second criterion is the one that matters. The reason this is an issue rather than a commit is precisely that the obvious fix depends on an untested assumption, and substituting one unverified premise for another is not progress.

ahmad added the

labels

2026-09-04 06:04:44 +00:00