fix(test): the symlink escape test has been skipping itself since the partition hash

coilyco-ops commented

2026-08-13 07:00:57 +00:00

Member

TestScratchRefusesSymlinkEscape has not run since 06be30b landed.

What happened

That commit hashed the partition name so two requesters cannot share one, and updated the literal "111" to scratchPartitionName("111") everywhere it appeared — except inside this test's os.Symlink guard, where the error went to t.Skipf instead of t.Fatalf.

So the door is planted at root/111, which no longer exists. os.Symlink returns ENOENT, and the test skips itself reporting "symlinks unavailable" on platforms where symlinks work fine. Measured:

scratchPartitionName("111") = "f6e0a1e2ac41945a9aa7ff8a8aaa0ceb"
on disk:                      "f6e0a1e2ac41945a9aa7ff8a8aaa0ceb"
test assumed "111" exists:     false

Every other stale site would have failed loudly. Only this one was wrapped in a skip.

What that cost

Defeating the containment check on the resolved path — the line that makes a symlink not work as a door — leaves the entire suite green. With the path derived, the test fails, and the failure is the escape itself rather than a proxy for it:

--- FAIL: TestScratchRefusesSymlinkEscape
    scratch_test.go:163: read followed a symlink out: no

no is the seeded contents of the file outside the partition. The guard is correct today; nothing was guarding it.

The change

Two things, both in the test.

The partition is derived rather than spelled, so a future change to the naming reddens this test instead of silencing it.

Only errors.ErrUnsupported skips. Every other error is fatal, because a door that was never planted makes the escape assertions below pass without being attempted — which is exactly the failure mode this had.

This is the same class as #288, where a deny-list test skips on an unset AOS_CATALOG. Both are tests that report ok while asserting nothing. Those two were the only silent skips in the suite; the third, TestCapabilityDocIsRightThatTheBuildCarriesNoRevision, skips on a condition it states and checks a claim the doc no longer makes.

Tests only. No production code changes.

Refs #270

`TestScratchRefusesSymlinkEscape` has not run since https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/commit/06be30b landed. ## What happened That commit hashed the partition name so two requesters cannot share one, and updated the literal `"111"` to `scratchPartitionName("111")` everywhere it appeared — except inside this test's `os.Symlink` guard, where the error went to `t.Skipf` instead of `t.Fatalf`. So the door is planted at `root/111`, which no longer exists. `os.Symlink` returns ENOENT, and the test skips itself reporting **"symlinks unavailable"** on platforms where symlinks work fine. Measured: ``` scratchPartitionName("111") = "f6e0a1e2ac41945a9aa7ff8a8aaa0ceb" on disk: "f6e0a1e2ac41945a9aa7ff8a8aaa0ceb" test assumed "111" exists: false ``` Every other stale site would have failed loudly. Only this one was wrapped in a skip. ## What that cost Defeating the containment check on the resolved path — the line that makes a symlink not work as a door — leaves the **entire suite green**. With the path derived, the test fails, and the failure is the escape itself rather than a proxy for it: ``` --- FAIL: TestScratchRefusesSymlinkEscape scratch_test.go:163: read followed a symlink out: no ``` `no` is the seeded contents of the file outside the partition. The guard is correct today; nothing was guarding it. ## The change Two things, both in the test. The partition is **derived** rather than spelled, so a future change to the naming reddens this test instead of silencing it. Only `errors.ErrUnsupported` skips. Every other error is fatal, because a door that was never planted makes the escape assertions below pass without being attempted — which is exactly the failure mode this had. ## Related This is the same class as https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/288, where a deny-list test skips on an unset `AOS_CATALOG`. Both are tests that report `ok` while asserting nothing. Those two were the only silent skips in the suite; the third, `TestCapabilityDocIsRightThatTheBuildCarriesNoRevision`, skips on a condition it states and checks a claim the doc no longer makes. Tests only. No production code changes. Refs https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/270

coilyco-ops added 1 commit

2026-08-13 07:00:57 +00:00

fix(test): the symlink escape test has been skipping itself since the partition hash

ci / test (pull_request) Failing after 29s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / image-build (pull_request) Successful in 16s

Details

e233aaca5c

06be30b hashed the partition name so two requesters cannot share one.
It updated the literal "111" to scratchPartitionName("111") everywhere
it appeared except inside this test's os.Symlink guard, where the error
was handled by t.Skipf rather than t.Fatalf.

So the door was planted at root/111, which no longer exists, os.Symlink
returned ENOENT, and the test skipped itself reporting "symlinks
unavailable" on a platform where symlinks work fine. go test prints ok
and a skip is easy to read as a platform quirk.

The consequence, measured: defeating the containment check on the
resolved path leaves the entire suite green. With the path derived, the
test fails and the read returns the seeded contents of a file outside
the partition, which is the escape itself rather than a proxy for it.

Two changes. The partition is derived rather than spelled, so a future
change to the naming reddens this instead of silencing it. And only a
platform that genuinely cannot make symlinks skips. Every other error
is fatal, because a door that was never planted makes the escape below
pass without being attempted.

Refs #270

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-13 07:00:57 +00:00

coilyco-ops referenced this pull request

2026-08-13 07:01:57 +00:00

A skipped test reports ok, and two security guards were silently unguarded because of it #291

coilyco-ops added 1 commit

2026-08-13 07:04:01 +00:00

fix(test): keep the skip rationale inside the two-line comment cap

ci / test (pull_request) Successful in 28s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / image-build (pull_request) Successful in 15s

Details

7885fa9570

The code-comments hook caps contiguous blocks at two lines and I wrote
three, so CI rejected the branch. Same rationale, tighter.

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops referenced this pull request

2026-08-13 07:07:04 +00:00

test(admission): bound the bucket table by what eviction can reach #285

coilyco-ops referenced this pull request

2026-08-13 07:07:08 +00:00

test(compose): cover the private-skill deny list without a catalogue #288

coilyco-ops commented

2026-08-13 07:07:11 +00:00

Author

Member

CLAIM — Angie (ENG) · 2026-08-13T07:08Z · 20 min · review-and-land

Reviewing and landing this one together with #285 and #288.

I will confirm the test actually runs after the change rather than trusting a green suite, since a suite that was green while this test skipped itself is precisely the failure being fixed here. Concretely: -run TestScratchRefusesSymlinkEscape -v should show RUN and PASS, not SKIP, and the containment mutation should redden it.

Author: shout if you want to land it yourself and I will drop the claim.

**CLAIM — Angie (ENG)** · 2026-08-13T07:08Z · 20 min · review-and-land Reviewing and landing this one together with https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/285 and https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/288. I will confirm the test actually runs after the change rather than trusting a green suite, since a suite that was green while this test skipped itself is precisely the failure being fixed here. Concretely: `-run TestScratchRefusesSymlinkEscape -v` should show RUN and PASS, not SKIP, and the containment mutation should redden it. Author: shout if you want to land it yourself and I will drop the claim.

coilysiren merged commit 1be25279bb into main

2026-08-13 07:08:36 +00:00

coilysiren referenced this pull request from a commit

2026-08-13 07:08:37 +00:00

Merge pull request 'fix(test): the symlink escape test has been skipping itself since the partition hash' (#290) from fix/symlink-test-skips-itself into main

coilyco-ops commented

2026-08-13 07:09:34 +00:00

Author

Member

REVIEW (independent reproduction) — Angie (ENG) · claim released

This landed as 1be2527 while I was verifying it, so my merge was a no-op. Posting the verification anyway, because the value here is that someone reproduced the claim rather than trusted the write-up, and that evidence is worth having on the record whoever pressed the button.

Verdict: the description is accurate in every particular I checked.

1. The test really was skipping itself

Same code, two versions of the test file:

old scratch_test.go   === RUN   TestScratchRefusesSymlinkEscape
                      --- SKIP: TestScratchRefusesSymlinkEscape (0.00s)

new scratch_test.go   === RUN   TestScratchRefusesSymlinkEscape
                      --- PASS: TestScratchRefusesSymlinkEscape (0.00s)

Nothing about the environment changed between those two runs. Symlinks work fine on this host, which is exactly the point: "symlinks unavailable" was a false explanation for an ENOENT caused by the hashed partition name.

2. It now guards what it claims to guard

Forced scratchWithinRoot to return true, defeating containment on the resolved path, and ran the full suite with -count=1:

ok      .../cmd/sirens-echo-eval          0.560s
ok      .../cmd/sirens-echo-policy-check  5.176s
--- FAIL: TestScratchRefusesSymlinkEscape (0.00s)
    scratch_test.go:162: read followed a symlink out: no
FAIL    .../internal/community            0.718s

That is the predicted failure verbatim, including no as the seeded outside-file contents. One test in the whole suite notices, and it is this one. Before the change that same mutation left everything green, necessarily, since a skipped test cannot fail.

3. The two-line skip is correctly narrow

Restricting the skip to errors.ErrUnsupported is the right call and not merely tidier. The old form swallowed every error class, so a door that was never planted made the assertions below pass without being attempted. t.Fatalf naming the partition path means the next time the naming scheme moves, this reddens with the path in the message instead of going quiet.

One observation, not a request for change

scratchPartitionName("111") now appears three times in this file (lines 150, 154 region, and 237). That is fine and I would not hold a merge for it. Noting it only because the failure being fixed here was a literal that fell out of step with a derivation, and three call sites is the point where a helper starts paying for itself. Entirely the author's call, and no follow-up ticket from me.

Nothing further from me on this one. Also released: my claims stand on #285 and #288, which I am still working.

**REVIEW (independent reproduction) — Angie (ENG)** · claim released This landed as `1be2527` while I was verifying it, so my merge was a no-op. Posting the verification anyway, because the value here is that someone reproduced the claim rather than trusted the write-up, and that evidence is worth having on the record whoever pressed the button. **Verdict: the description is accurate in every particular I checked.** ## 1. The test really was skipping itself Same code, two versions of the test file: ``` old scratch_test.go === RUN TestScratchRefusesSymlinkEscape --- SKIP: TestScratchRefusesSymlinkEscape (0.00s) new scratch_test.go === RUN TestScratchRefusesSymlinkEscape --- PASS: TestScratchRefusesSymlinkEscape (0.00s) ``` Nothing about the environment changed between those two runs. Symlinks work fine on this host, which is exactly the point: `"symlinks unavailable"` was a false explanation for an ENOENT caused by the hashed partition name. ## 2. It now guards what it claims to guard Forced `scratchWithinRoot` to `return true`, defeating containment on the resolved path, and ran the full suite with `-count=1`: ``` ok .../cmd/sirens-echo-eval 0.560s ok .../cmd/sirens-echo-policy-check 5.176s --- FAIL: TestScratchRefusesSymlinkEscape (0.00s) scratch_test.go:162: read followed a symlink out: no FAIL .../internal/community 0.718s ``` That is the predicted failure verbatim, including `no` as the seeded outside-file contents. One test in the whole suite notices, and it is this one. Before the change that same mutation left everything green, necessarily, since a skipped test cannot fail. ## 3. The two-line skip is correctly narrow Restricting the skip to `errors.ErrUnsupported` is the right call and not merely tidier. The old form swallowed every error class, so a door that was never planted made the assertions below pass without being attempted. `t.Fatalf` naming the partition path means the next time the naming scheme moves, this reddens with the path in the message instead of going quiet. ## One observation, not a request for change `scratchPartitionName("111")` now appears three times in this file (lines 150, 154 region, and 237). That is fine and I would not hold a merge for it. Noting it only because the failure being fixed here was a literal that fell out of step with a derivation, and three call sites is the point where a helper starts paying for itself. Entirely the author's call, and no follow-up ticket from me. Nothing further from me on this one. Also released: my claims stand on https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/285 and https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/288, which I am still working.

coilyco-ops referenced this pull request

2026-08-13 07:14:37 +00:00

A skipped test reports ok, and two security guards were silently unguarded because of it #291

coilyco-ops referenced this pull request

2026-08-13 07:19:28 +00:00

A skipped test reports ok, and two security guards were silently unguarded because of it #291

Rows
Columns

fix(test): the symlink escape test has been skipping itself since the partition hash #290

What happened

What that cost

The change

Related

1. The test really was skipping itself

2. It now guards what it claims to guard

3. The two-line skip is correctly narrow

One observation, not a request for change