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

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

ahmad self-assigned this

2026-09-07 15:44:04 +00:00

ahmad added

state/in-progress

and removed

state/ready

labels

2026-09-07 15:44:05 +00:00

ahmad commented

2026-09-07 15:44:05 +00:00

Author

Owner

Plan, honouring the issue's ordering: evidence first. The WIP PR opens with a temporary probe step in validate that prints runner.temp, RUNNER_TEMP, the job's container identity (os-release, hostname) and /tmp's device — the runner's own CI run then answers criteria 1 and 2, and the findings land here as a comment before any path changes. Implementation follows the evidence: if runner.temp is confirmed non-empty and per-job, the action's log path moves to it with the output expression kept in lockstep (composite outputs evaluate runner context without a GITHUB_OUTPUT step, preserving the deliberate literal-output shape); a shell-side guard keeps /tmp as fallback so an empty expansion can never produce /security-scan.log. The probe commit is dropped before the PR leaves WIP.

ahmad referenced this issue from a commit

2026-09-07 15:44:20 +00:00

chore: Probe the runner temp model (temporary)

ahmad referenced this issue

2026-09-07 15:44:41 +00:00

WIP: chore: Make the security-scan log path safe per job #55

ahmad commented

2026-09-07 16:13:56 +00:00

Author

Owner

Findings from the probe run (platform-actions run 2432, validate job — the probe step is in that log verbatim):

Criterion 1 — the runner's filesystem model: /tmp is isolated per job. The job executed inside its own docker container (docker exec, container hostname 25c4a0f7d05e, overlay mounted at /), and its /tmp contained exactly one file: the marker the probe had just written. Nothing from any other job, ever. This is act_runner's label→image model: every job — with or without a container: of its own — runs in a per-job container. Corroborating operational evidence: ahmad/expiro's image job (no container:) installs the docker CLI with apt from scratch every run and its /tmp/image-job.log has never collided across the hundreds of runs this week.

Criterion 2 — ${{ runner.temp }} expands non-empty: it expands to exactly /tmp (and RUNNER_TEMP=/tmp in the environment). So the "obvious fix" — substituting the expression for the constant — would be a semantic no-op on this runner: same path, same isolation, one more layer of indirection.

Conclusion: the scope's conditional ("if it does not isolate, make the path unique") does not fire. The fixed /tmp/security-scan.log is safe because isolation comes from the per-job container, not from the path — and criteria 3 and 4 hold by construction under that model. The interleaving scenario would require the runner to switch to host-mode execution, which would break far more than this log (every apt-get install in every workflow); if that ever changes, this issue's probe is the two-minute way to re-answer.

Closing: the issue asked for an answer before a change, and the answer is that no change is warranted. The probe commit is discarded and the WIP PR closed without merging.

Findings from the probe run (platform-actions run 2432, validate job — the probe step is in that log verbatim): **Criterion 1 — the runner's filesystem model: /tmp is isolated per job.** The job executed inside its own docker container (`docker exec`, container hostname `25c4a0f7d05e`, `overlay` mounted at `/`), and its `/tmp` contained exactly one file: the marker the probe had just written. Nothing from any other job, ever. This is act_runner's label→image model: every job — with or without a `container:` of its own — runs in a per-job container. Corroborating operational evidence: `ahmad/expiro`'s `image` job (no `container:`) installs the docker CLI with apt from scratch every run and its `/tmp/image-job.log` has never collided across the hundreds of runs this week. **Criterion 2 — `${{ runner.temp }}` expands non-empty:** it expands to exactly `/tmp` (and `RUNNER_TEMP=/tmp` in the environment). So the "obvious fix" — substituting the expression for the constant — would be a semantic no-op on this runner: same path, same isolation, one more layer of indirection. **Conclusion:** the scope's conditional ("if it does not isolate, make the path unique") does not fire. The fixed `/tmp/security-scan.log` is safe because isolation comes from the per-job container, not from the path — and criteria 3 and 4 hold by construction under that model. The interleaving scenario would require the runner to switch to host-mode execution, which would break far more than this log (every apt-get install in every workflow); if that ever changes, this issue's probe is the two-minute way to re-answer. Closing: the issue asked for an answer before a change, and the answer is that no change is warranted. The probe commit is discarded and the WIP PR closed without merging.

ahmad referenced this issue

2026-09-07 16:13:56 +00:00

WIP: chore: Make the security-scan log path safe per job #55

ahmad

2026-09-07 16:13:58 +00:00