rastrillo / fiesta Public

Resolve a linked -out and refuse links inside the tree before emptying

claim resolves -out with filepath.EvalSymlinks before it reads the
stamp, walks the tree or empties anything, so every step sees the
same real directory. Before this, ReadDir and ReadFile followed a
link while WalkDir did not: fiesta gallery pointed at a link to a
gallery directory with a release built inside passed the stamp
checks, found nothing below, and emptied the release.
TestGalleryRefusesALinkedDirectoryHoldingARelease runs exactly that
(gallery, build, ln -s, gallery through the link) and was red against
8c2c122 before the change: "fiesta gallery emptied .../site through
the link .../link".

A symbolic link anywhere inside the tree is refused by name, whatever
it points at. Neither command writes one, so it is not their output,
and refusing every link is the one rule that both never follows a
link out of the tree when emptying and never empties a
foreign-stamped directory through one; the comment on claim says so.
TestGalleryRefusesALinkInsideItsDirectory links a release into a
gallery directory and was red against 8c2c122 too, the run exiting 0
with the link removed.

stampBelow becomes foreignBelow, one read-only walk that returns the
refusal for the first stamp other than the root's or the first link,
and nothing is removed until it returns nil.

Three tests from the last round had no recorded red run; each now has
one from a scratch mutation of 8c2c122: sample hrefs and actions
rewritten to the mount root instead of # reddens
TestSampleURLsStayInTheMarkup ("<a href=\"/x/\"> is not a fragment"),
the unrendered other heading left as a placeholder reddens
TestTheGalleryCarriesNoPlaceholderCopy through the table check, and
each half of the right-to-left rewrite dropped in turn reddens
TestRTLDocumentsReadRightToLeft, frames and pages respectively.

Human

Prompt

Fix the symlinked -out bypass the re-review found: resolve -out with EvalSymlinks before checking, walking and emptying; decide and state the rule for links inside the tree; add a test for the reviewer's exact scenario, red before and green after. Record red runs for the three tests that lacked one.

From this commit's own message. The prompt ledger has no entries for it — which is what a squashed, rebased or amended commit looks like, and what every commit looks like in a repo without a ledger.

Summary

claim resolves a linked -out first and refuses any symbolic link inside the tree; two new tests for the linked root and a link inside; red runs recorded for the mount, placeholder-map and RTL tests.

Agent notes

The link refusal message is new CLI text for the same copy batch as the other refusals. A link in the parent chain of an absent -out is followed by MkdirAll, which is the only place a link is followed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

claude 1 week ago aa5ccb602af89e50f71d08e1b031d6757e54b390 parent 8c2c122

2 files changed, +106 −14

cmd/fiesta/claim.go +31 −14
cmd/fiesta/claim_test.go +75 −0

CI passed — run details

CI log

=== gofmt ===
=== no-rastrillo ===
go test -run '^(TestFiestaNeverImportsRastrillo|TestThePublicPackagesLinkNoBrowser)$' -count=1 .
ok  	amadan.net/rastrillo/fiesta	0.091s
=== size ===
go test ./internal/build -run '^TestTheOneLinkIsUnder25KiBGzipped$' -count=1 -v
=== RUN   TestTheOneLinkIsUnder25KiBGzipped
    build_test.go:137: fiesta-plain.css: 152856 bytes, 22303 bytes gzipped (21.8 KiB of the 25.0 KiB limit)
    build_test.go:137: fiesta-signal.css: 152951 bytes, 22414 bytes gzipped (21.9 KiB of the 25.0 KiB limit)
    build_test.go:137: fiesta.css: 155770 bytes, 23018 bytes gzipped (22.5 KiB of the 25.0 KiB limit)
--- PASS: TestTheOneLinkIsUnder25KiBGzipped (0.02s)
PASS
ok  	amadan.net/rastrillo/fiesta/internal/build	0.021s
=== suite ===
--- shard 0 of 10 ---
==> shard 0 of 10: make root
go build ./...
go vet ./...
FIESTA_TEST_REQUIRE_NODE=1 go test -count=1 ./...
ok  	amadan.net/rastrillo/fiesta	0.337s
ok  	amadan.net/rastrillo/fiesta/check	3.511s
ok  	amadan.net/rastrillo/fiesta/cmd/fiesta	3.400s
ok  	amadan.net/rastrillo/fiesta/example	0.111s
ok  	amadan.net/rastrillo/fiesta/internal/axe	0.087s
ok  	amadan.net/rastrillo/fiesta/internal/build	0.908s
ok  	amadan.net/rastrillo/fiesta/internal/codeview	0.018s
ok  	amadan.net/rastrillo/fiesta/internal/gallery	1.926s
?   	amadan.net/rastrillo/fiesta/internal/harness	[no test files]
ok  	amadan.net/rastrillo/fiesta/internal/markup	0.010s
ok  	amadan.net/rastrillo/fiesta/internal/nodetest	0.377s
ok  	amadan.net/rastrillo/fiesta/internal/tokens	0.012s
--- shard 1 of 10 ---
==> shard 1 of 10: make staticcheck
go run honnef.co/go/tools/cmd/staticcheck@v0.8.2 -tags browser ./...
--- shard 2 of 10 ---
==> shard 2 of 10: make govulncheck
go run golang.org/x/vuln/cmd/govulncheck@v1.8.0 ./...
=== Symbol Results ===

No vulnerabilities found.

Your code is affected by 0 vulnerabilities.
This scan also found 0 vulnerabilities in packages you import and 5
vulnerabilities in modules you require, but your code doesn't appear to call
these vulnerabilities.
Use '-show verbose' for more details.
--- shard 3 of 10 ---
==> shard 3 of 10: make gitleaks
go run github.com/zricethezav/gitleaks/v8@v8.30.1 git --no-banner --redact --log-opts='--full-history --diff-filter=tuxdb HEAD' .
[90m8:39PM[0m [32mINF[0m [1mUnknown SCM platform. Use --platform to include links in findings.[0m [36mhost=[0mamadan.net
[90m8:39PM[0m [32mINF[0m [1m1 commits scanned.[0m
[90m8:39PM[0m [32mINF[0m [1mscanned ~4495966 bytes (4.50 MB) in 2.26s[0m
[90m8:39PM[0m [32mINF[0m [1mno leaks found[0m
--- shard 4 of 10 ---
==> shard 4 of 10: make browser.internal.harness
./hack/gotest.sh browser 'internal.harness'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
ok  	amadan.net/rastrillo/fiesta/internal/harness	9.154s
--- shard 5 of 10 ---
==> shard 5 of 10: make browser.check~1of3
./hack/gotest.sh browser 'check~1of3'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 1 of 3 of ./check/ runs 92 of 276 tests
ok  	amadan.net/rastrillo/fiesta/check	190.096s
--- shard 6 of 10 ---
==> shard 6 of 10: make browser.check~2of3
./hack/gotest.sh browser 'check~2of3'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 2 of 3 of ./check/ runs 92 of 276 tests
ok  	amadan.net/rastrillo/fiesta/check	194.967s
--- shard 7 of 10 ---
==> shard 7 of 10: make browser.check~3of3
./hack/gotest.sh browser 'check~3of3'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 3 of 3 of ./check/ runs 92 of 276 tests
ok  	amadan.net/rastrillo/fiesta/check	193.343s
--- shard 8 of 10 ---
==> shard 8 of 10: make browser.internal.gallery~1of2
./hack/gotest.sh browser 'internal.gallery~1of2'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 1 of 2 of ./internal/gallery/ runs 1 of 13 tests
ok  	amadan.net/rastrillo/fiesta/internal/gallery	67.127s
--- shard 9 of 10 ---
==> shard 9 of 10: make browser.internal.gallery~2of2
./hack/gotest.sh browser 'internal.gallery~2of2'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 2 of 2 of ./internal/gallery/ runs 12 of 13 tests
ok  	amadan.net/rastrillo/fiesta/internal/gallery	18.976s

diff --git a/cmd/fiesta/claim.go b/cmd/fiesta/claim.go

index feb7d18..67a2d44 100644

--- a/cmd/fiesta/claim.go

+++ b/cmd/fiesta/claim.go

@@ -65,11 +65,27 @@ func stampBody(command string) string {

// there is never a second copy to drift: the output is the build,

// every time.

+// Symbolic links, two rules. A root that is a link is resolved first,

+// so reading the stamp, walking the tree and emptying it all see the

+// same real directory: ReadDir and ReadFile follow a link and WalkDir

+// does not, and a gallery pointed at a link to a directory with a

+// release inside passed the check and emptied the release. A link

+// anywhere inside the tree is refused by name, whatever it points at:

+// neither command writes one, so it is not their output, and refusing

+// it is the one rule that both never follows a link out of the tree

+// when emptying and never empties a foreign-stamped directory through

+// one.

+//

// The stamp is written before the files, so a run interrupted halfway

// leaves a directory the command still owns and will clean out on the

// next run. It is not atomic: a failed run leaves a partial tree

// behind, and the only thing saying so is the non-zero exit status.

func claim(root, command string) error {

+ if real, err := filepath.EvalSymlinks(root); err == nil {

+ root = real

+ } else if !os.IsNotExist(err) {

+ return fmt.Errorf("resolving %s: %w", root, err)

+ }

entries, err := os.ReadDir(root)

switch {

case os.IsNotExist(err):

@@ -93,11 +109,8 @@ func claim(root, command string) error {

return fmt.Errorf("refusing to empty %s: its %s stamp says it is written by %s, and %s would empty that command's output. "+

"Point -out somewhere else", root, stamp, by, command)

}

- if other, err := stampBelow(root); err != nil {

- return fmt.Errorf("reading %s: %w", root, err)

- } else if other != "" {

- return fmt.Errorf("refusing to empty %s: %s is a stamp %s did not write, so another command's output is inside. "+

- "Empty it yourself, or point -out somewhere else", root, other, command)

+ if err := foreignBelow(root, command); err != nil {

+ return err

}

for _, e := range entries {

if err := os.RemoveAll(filepath.Join(root, e.Name())); err != nil {

@@ -111,19 +124,23 @@ func claim(root, command string) error {

return nil

}

-// stampBelow returns the path of the first stamp under root other than

-// root's own, or "" when there is none.

-func stampBelow(root string) (string, error) {

- found := ""

- err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error {

+// foreignBelow walks root and refuses the first thing under it that

+// is not the command's own output: a stamp other than root's, or a

+// symbolic link. It only reads; nothing is removed until it returns

+// nil. An unreadable directory is an error, and so a refusal.

+func foreignBelow(root, command string) error {

+ return filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error {

if err != nil {

- return err

+ return fmt.Errorf("reading %s: %w", root, err)

+ }

+ if d.Type()&fs.ModeSymlink != 0 {

+ return fmt.Errorf("refusing to empty %s: %s is a symbolic link, which %s never writes, so it is not this command's output. "+

+ "Remove it yourself, or point -out somewhere else", root, p, command)

}

if !d.IsDir() && d.Name() == stampName && filepath.Dir(p) != root {

- found = p

- return fs.SkipAll

+ return fmt.Errorf("refusing to empty %s: %s is a stamp %s did not write, so another command's output is inside. "+

+ "Empty it yourself, or point -out somewhere else", root, p, command)

}

return nil

})

- return found, err

}

diff --git a/cmd/fiesta/claim_test.go b/cmd/fiesta/claim_test.go

index aec232a..3839795 100644

--- a/cmd/fiesta/claim_test.go

+++ b/cmd/fiesta/claim_test.go

@@ -179,3 +179,78 @@ func TestGalleryRefusesADirectoryHoldingARelease(t *testing.T) {

t.Errorf("the refused gallery directory was touched: %v", err)

}

+// TestGalleryRefusesALinkedDirectoryHoldingARelease: the reviewer's

+// route round the check. A symbolic link to a gallery directory that

+// a release has been built inside is refused like the directory

+// itself, naming the release's stamp by its real path, and the release

+// is left as it was. The link is resolved before anything is read,

+// walked or emptied, so every step sees the same real directory.

+func TestGalleryRefusesALinkedDirectoryHoldingARelease(t *testing.T) {

+ base := t.TempDir()

+ site := filepath.Join(base, "site")

+ if err := fiestaGallery(t, site); err != nil {

+ t.Fatal(err)

+ }

+ if err := fiestaBuild(t, site); err != nil {

+ t.Fatal(err)

+ }

+ link := filepath.Join(base, "link")

+ if err := os.Symlink(site, link); err != nil {

+ t.Fatal(err)

+ }

+ release := filepath.Join(site, fiesta.Version)

+ err := fiestaGallery(t, link)

+ if err == nil {

+ t.Fatalf("fiesta gallery emptied %s through the link %s, and with it the release %s", site, link, release)

+ }

+ real, err2 := filepath.EvalSymlinks(filepath.Join(release, stampName))

+ if err2 != nil {

+ t.Fatalf("the release's stamp is gone: %v", err2)

+ }

+ if !strings.Contains(err.Error(), real) {

+ t.Errorf("the refusal does not name %s: %v", real, err)

+ }

+ if _, err := os.Stat(filepath.Join(release, "fiesta.css")); err != nil {

+ t.Errorf("the release lost fiesta.css: %v", err)

+ }

+// TestGalleryRefusesALinkInsideItsDirectory: a symbolic link inside a

+// directory the command owns is refused by name, whatever it points

+// at, and what it points at is left alone. The commands never write a

+// link, so one inside their output is not theirs; refusing every link

+// is the one rule that both never follows a link out of the tree and

+// never empties a foreign-stamped directory through one.

+func TestGalleryRefusesALinkInsideItsDirectory(t *testing.T) {

+ base := t.TempDir()

+ site := filepath.Join(base, "site")

+ if err := fiestaGallery(t, site); err != nil {

+ t.Fatal(err)

+ }

+ dist := filepath.Join(base, "dist")

+ if err := fiestaBuild(t, dist); err != nil {

+ t.Fatal(err)

+ }

+ release := filepath.Join(dist, fiesta.Version)

+ link := filepath.Join(site, "release")

+ if err := os.Symlink(release, link); err != nil {

+ t.Fatal(err)

+ }

+ err := fiestaGallery(t, site)

+ if err == nil {

+ t.Fatalf("fiesta gallery emptied %s with the link %s in it", site, link)

+ }

+ if !strings.Contains(err.Error(), link) {

+ t.Errorf("the refusal does not name the link %s: %v", link, err)

+ }

+ if _, err := os.Lstat(link); err != nil {

+ t.Errorf("the link was removed: %v", err)

+ }

+ if _, err := os.Stat(filepath.Join(release, "fiesta.css")); err != nil {

+ t.Errorf("the release behind the link lost fiesta.css: %v", err)

+ }

+ if _, err := os.Stat(filepath.Join(site, "index.html")); err != nil {

+ t.Errorf("the refused directory was touched: %v", err)

+ }