Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions internal/guard/boundaryresolution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ func TestALinkInsideTheDirectoryPointingBackInsideItIsNotAnEscape(t *testing.T)
t.Fatalf("making the directories: %v", err)
}
out := filepath.Join(root, "out")
if err := os.Symlink(actual, out); err != nil {
t.Skipf("this system will not create a link here: %v", err)
}
plantLink(t, actual, out)

if code, _, errOut := run(t,
"generate", "--format", "txt", "--size", "1kb", "--count", "1",
Expand All @@ -78,9 +76,7 @@ func TestALinkInsideTheDirectoryPointingBackInsideItIsNotAnEscape(t *testing.T)
if err := os.Rename(named, target); err != nil {
t.Fatalf("moving the file: %v", err)
}
if err := os.Symlink(target, named); err != nil {
t.Skipf("this system will not create a link here: %v", err)
}
plantLink(t, target, named)

mf := filepath.Join(out, "manifest.json")
code, stdout, errOut := run(t, "verify", mf)
Expand Down
4 changes: 1 addition & 3 deletions internal/guard/safety_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,7 @@ func TestAFreshRunIntoAnEmptyDirectoryStillWorks(t *testing.T) {
func TestANameTakenByALinkPointingNowhereIsStillTaken(t *testing.T) {
dir := t.TempDir()
dangling := filepath.Join(dir, "files_0001.txt")
if err := os.Symlink(filepath.Join(dir, "nothing-is-here"), dangling); err != nil {
t.Skipf("this system will not create a link here, so the case cannot be built: %v", err)
}
plantLink(t, filepath.Join(dir, "nothing-is-here"), dangling)

// Asserted rather than assumed, because the whole case is a name that IS
// there and that os.Stat cannot see. A fixture that quietly resolved would
Expand Down
143 changes: 136 additions & 7 deletions internal/guard/symlinkescape_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package guard

import (
"errors"
"io/fs"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"syscall"
"testing"

"github.com/donislawdev/TestingFilesGenerator/internal/cli"
Expand All @@ -29,7 +32,7 @@ import (
//
// The fixture is built with os.Symlink rather than a junction so that the same
// guard runs on all three systems in the matrix. Windows refuses to create one
// without the privilege, and the guard says so rather than passing quietly -
// without the privilege, and plantLink says so rather than passing quietly -
// a skip that looks like a pass is how this class of defect survives.
func linkedEscape(t *testing.T) (out, victim string) {
t.Helper()
Expand All @@ -42,12 +45,140 @@ func linkedEscape(t *testing.T) (out, victim string) {
if err := os.WriteFile(victim, []byte("the owner's own work\n"), 0o644); err != nil {
t.Fatalf("writing the victim: %v", err)
}
if err := os.Symlink(root, filepath.Join(out, "jn")); err != nil {
t.Skipf("this system will not create a link here, so the escape cannot be built: %v", err)
}
plantLink(t, root, filepath.Join(out, "jn"))
return out, victim
}

// plantLink makes link lead to target, or says out loud why the case cannot
// run.
//
// Creating a symbolic link needs a privilege on Windows that an ordinary
// account does not have. Off CI that is a skip, which -v prints. On CI it is a
// failure, because the Windows job runs go test without -v, and there a skip
// reads exactly like a pass. On 2026-09-29 the guard asked to prove that a
// directory reached through a link still works after O252 was green on all
// three systems, and nothing said whether it had run on Windows at all. The
// same guard skipped on the machine that wrote it, with "A required privilege
// is not held by the client". The first CI run after this helper was green on
// Windows, and GitHub sets CI on every job, so the runner makes links.
func plantLink(t *testing.T, target, link string) {
t.Helper()
plantLinkWith(t, os.Symlink, os.Getenv("CI"), target, link)
}

// linkReporter is the part of testing.T that plantLinkWith speaks to, so that
// its decision can be asked of a recorder rather than of a test that really
// stops.
type linkReporter interface {
Helper()
Fatalf(format string, args ...any)
Skipf(format string, args ...any)
}

// plantLinkWith is plantLink with the two things it reads from the world
// passed in. A real testing.T stops at Fatalf and Skipf and a recorder does
// not, so every branch returns on its own.
func plantLinkWith(t linkReporter, symlink func(oldname, newname string) error, ci, target, link string) {
t.Helper()
err := symlink(target, link)
if err == nil {
return
}
if !linkWantsAPrivilege(err) {
t.Fatalf("planting a symbolic link %s to %s: %v", link, target, err)
return
}
if !linkCasesMaySkip(ci) {
t.Fatalf("this host does not allow creating a symbolic link (%v), so this case did not run.\n"+
"On CI a case that did not run must not look like one that passed. "+
"Give the job the privilege, or build this case with something that needs none.", err)
return
}
t.Skipf("this host does not allow creating a symbolic link (%v), so this case did not run", err)
}

// errPrivilegeNotHeld is what Windows answers an account that may not create a
// symbolic link. By number, because the syscall package does not name it and
// its text is written in the system's own language.
const errPrivilegeNotHeld = syscall.Errno(1314) // ERROR_PRIVILEGE_NOT_HELD

// linkWantsAPrivilege says whether a link was refused for want of a privilege.
// Asked of the error inside rather than of the text: the text of an
// os.LinkError carries both paths, and a path is not evidence.
func linkWantsAPrivilege(err error) bool {
return errors.Is(err, errPrivilegeNotHeld) || errors.Is(err, fs.ErrPermission)
}

// linkCasesMaySkip says whether a case built on a symbolic link may skip when
// the host refuses to make one.
//
// A function of its input for the reason screensAreCompared is one: a skipped
// test is a green test, so a condition widened by accident stops the case from
// being checked without one thing going red. Asked this way, the CI answer is
// tested on a machine that is not CI.
func linkCasesMaySkip(ci string) bool {
return ci == ""
}

// linkRecorder answers for testing.T in TestACaseBuiltOnALinkSkipsOnlyOffCI.
type linkRecorder struct{ failed, skipped bool }

func (r *linkRecorder) Helper() {}
func (r *linkRecorder) Fatalf(string, ...any) { r.failed = true }
func (r *linkRecorder) Skipf(string, ...any) { r.skipped = true }

func (r *linkRecorder) outcome() string {
switch {
case r.failed && r.skipped:
return "failed and skipped"
case r.failed:
return "failed"
case r.skipped:
return "skipped"
}
return "made"
}

// What this defends. A case built on a symbolic link runs on every CI system
// or fails there. It never skips there, because nothing would show it did.
// And only a refused privilege is a skip anywhere - any other refusal is a
// failure, whatever the paths in it say.
//
// Asked of plantLinkWith rather than of the predicate alone (outside review of
// #151): a guard on linkCasesMaySkip stays green if plantLink stops asking it.
func TestACaseBuiltOnALinkSkipsOnlyOffCI(t *testing.T) {
refused := &os.LinkError{Op: "symlink", Old: "target", New: "link", Err: errPrivilegeNotHeld}
denied := &os.LinkError{Op: "symlink", Old: "target", New: "link", Err: fs.ErrPermission}
// The paths say "privilege" and the error inside does not.
other := &os.LinkError{Op: "symlink", Old: "privilege", New: "privilege", Err: fs.ErrExist}

cases := []struct {
err error
ci string
want string
why string
}{
{nil, "true", "made", "a link that was made needs nothing said about it"},
{refused, "", "skipped", "off CI a Windows account without the privilege is not a defect, and -v prints the skip"},
{refused, "true", "failed", "on CI the Windows job runs without -v, so a skip there reads as a pass"},
{denied, "", "skipped", "a Unix permission refusal is the same case as the Windows privilege"},
{denied, "true", "failed", "on CI no refusal may hide a case"},
{other, "", "failed", "a refusal that is not about the privilege is a defect, whatever the paths say"},
{other, "true", "failed", "a refusal that is not about the privilege is a defect on CI too"},
}
for _, c := range cases {
r := &linkRecorder{}
plantLinkWith(r, func(string, string) error { return c.err }, c.ci, "target", "link")
if got := r.outcome(); got != c.want {
t.Errorf("with %v and CI=%q the case %s, want %s.\n"+
"Reason: %s.\n"+
"What to do: a skip is green, so a wider skip stops the link cases from being checked "+
"without anything going red. Narrow it back.",
c.err, c.ci, got, c.want, c.why)
}
}
}

// A junction is the other redirection Windows offers, and it is the one that
// matters most here: creating it needs no privilege at all, while a symbolic
// link does.
Expand Down Expand Up @@ -163,9 +294,7 @@ func TestADirectoryReachedThroughALinkStillWorks(t *testing.T) {
t.Fatalf("making the directory: %v", err)
}
linked := filepath.Join(root, "linked")
if err := os.Symlink(real, linked); err != nil {
t.Skipf("this system will not create a link here: %v", err)
}
plantLink(t, real, linked)

// Generate through the link, then verify and clean up through it. Every
// path involved resolves inside, so all three have to behave normally.
Expand Down
30 changes: 5 additions & 25 deletions internal/guard/writeescape_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,13 @@ func TestCreateNewCreatesOnlyWhenTheNameIsFree(t *testing.T) {
t.Fatalf("planting a hard link: %v", err)
}
}},
// The hard link case above needs no privilege anywhere, so the shape
// that matters most is never the one plantLink skips off CI.
{"a symbolic link to a file outside the directory", func(t *testing.T, dir, name, victim string) {
if err := os.Symlink(victim, name); err != nil {
skipIfLinksAreNotAllowed(t, err)
}
plantLink(t, victim, name)
}},
{"a symbolic link to nothing at all", func(t *testing.T, dir, name, victim string) {
if err := os.Symlink(filepath.Join(dir, "nothing-is-here"), name); err != nil {
skipIfLinksAreNotAllowed(t, err)
}
plantLink(t, filepath.Join(dir, "nothing-is-here"), name)
}},
}

Expand Down Expand Up @@ -265,9 +263,7 @@ func TestAHeldTemporaryNameStopsTheWriteRatherThanGoingThroughIt(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "manifest.json")
target := filepath.Join(t.TempDir(), "created-by-escape.json")
if err := os.Symlink(target, path); err != nil {
skipIfLinksAreNotAllowed(t, err)
}
plantLink(t, target, path)

if r, err := manifest.Claim(path); err == nil {
_ = r.Release()
Expand Down Expand Up @@ -311,19 +307,3 @@ func TestAHeldTemporaryNameStopsTheWriteRatherThanGoingThroughIt(t *testing.T) {
}
})
}

// skipIfLinksAreNotAllowed says out loud when a case did not run.
//
// Creating a symbolic link needs a privilege on Windows that an ordinary
// account does not have, and a case that quietly passes because it never ran is
// the failure this project has recorded more than any other. The hard link
// cases above need no privilege anywhere, so the shape that matters most is
// never the one being skipped.
func skipIfLinksAreNotAllowed(t *testing.T, err error) {
t.Helper()
if errors.Is(err, fs.ErrPermission) || strings.Contains(err.Error(), "privilege") {
t.Skipf("this host does not allow creating a symbolic link (%v), so this case did not run. "+
"The hard link cases beside it did.", err)
}
t.Fatalf("planting a symbolic link: %v", err)
}
Loading