detached d3bf1f1

fix(snapshot): reject symlinked repository roots

commit
d3bf1f1a4a4828880ea60c22f427411a7ae8dd69
author
brice · · 2026-07-22 09:33 UTC
parent
0d68d26
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
3 files · +74 -8
Unified
UnifiedSplit
M internal/adapters/snapshot/path_security_test.go +43 -0
@@ -11,6 +11,49 @@ import (
1111 "testing"
1212 )
1313
14+func TestRepositoryRejectsSymlinkedRoot(t *testing.T) {
15+ configuredRoot := filepath.Join(t.TempDir(), "configured")
16+ externalRoot := privateDir(t)
17+ if err := os.Symlink(externalRoot, configuredRoot); err != nil {
18+ t.Fatal(err)
19+ }
20+ if _, err := NewRepository(configuredRoot).openRoot(); err == nil {
21+ t.Fatal("openRoot succeeded through configured-root symlink")
22+ }
23+
24+ configuredRoot = privateDir(t)
25+ repo := NewRepository(configuredRoot)
26+ publication := repositoryPublication(t, "named", 1, []byte("configured"))
27+ if err := repo.Publish(context.Background(), publication); err != nil {
28+ t.Fatal(err)
29+ }
30+ external := NewRepository(externalRoot)
31+ externalPublication := repositoryPublication(t, publication.Name, 1, []byte("external"))
32+ if err := external.Publish(context.Background(), externalPublication); err != nil {
33+ t.Fatal(err)
34+ }
35+ if err := os.Rename(configuredRoot, configuredRoot+"-real"); err != nil {
36+ t.Fatal(err)
37+ }
38+ if err := os.Symlink(externalRoot, configuredRoot); err != nil {
39+ t.Fatal(err)
40+ }
41+
42+ if _, err := repo.Load(context.Background(), publication.Name); err == nil {
43+ t.Fatal("Load succeeded through replaced configured-root symlink")
44+ }
45+ if err := repo.Maintain(context.Background()); err == nil {
46+ t.Fatal("Maintain succeeded through replaced configured-root symlink")
47+ }
48+ loaded, err := external.Load(context.Background(), publication.Name)
49+ if err != nil {
50+ t.Fatal(err)
51+ }
52+ if string(loaded.Manifest) != string(externalPublication.Manifest) {
53+ t.Fatal("external repository changed")
54+ }
55+}
56+
1457 func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {
1558 for _, target := range []string{repositoryGenerations, repositoryObjectsDir} {
1659 t.Run(target, func(t *testing.T) {
@@ -11,6 +11,49 @@ import (
11 "testing"11 "testing"
12 )12 )
13 13
14+func TestRepositoryRejectsSymlinkedRoot(t *testing.T) {
15+ configuredRoot := filepath.Join(t.TempDir(), "configured")
16+ externalRoot := privateDir(t)
17+ if err := os.Symlink(externalRoot, configuredRoot); err != nil {
18+ t.Fatal(err)
19+ }
20+ if _, err := NewRepository(configuredRoot).openRoot(); err == nil {
21+ t.Fatal("openRoot succeeded through configured-root symlink")
22+ }
23+
24+ configuredRoot = privateDir(t)
25+ repo := NewRepository(configuredRoot)
26+ publication := repositoryPublication(t, "named", 1, []byte("configured"))
27+ if err := repo.Publish(context.Background(), publication); err != nil {
28+ t.Fatal(err)
29+ }
30+ external := NewRepository(externalRoot)
31+ externalPublication := repositoryPublication(t, publication.Name, 1, []byte("external"))
32+ if err := external.Publish(context.Background(), externalPublication); err != nil {
33+ t.Fatal(err)
34+ }
35+ if err := os.Rename(configuredRoot, configuredRoot+"-real"); err != nil {
36+ t.Fatal(err)
37+ }
38+ if err := os.Symlink(externalRoot, configuredRoot); err != nil {
39+ t.Fatal(err)
40+ }
41+
42+ if _, err := repo.Load(context.Background(), publication.Name); err == nil {
43+ t.Fatal("Load succeeded through replaced configured-root symlink")
44+ }
45+ if err := repo.Maintain(context.Background()); err == nil {
46+ t.Fatal("Maintain succeeded through replaced configured-root symlink")
47+ }
48+ loaded, err := external.Load(context.Background(), publication.Name)
49+ if err != nil {
50+ t.Fatal(err)
51+ }
52+ if string(loaded.Manifest) != string(externalPublication.Manifest) {
53+ t.Fatal("external repository changed")
54+ }
55+}
56+
14 func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {57 func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {
15 for _, target := range []string{repositoryGenerations, repositoryObjectsDir} {58 for _, target := range []string{repositoryGenerations, repositoryObjectsDir} {
16 t.Run(target, func(t *testing.T) {59 t.Run(target, func(t *testing.T) {
M internal/adapters/snapshot/publish_efficiency_test.go +6 -7
@@ -35,18 +35,17 @@ func TestRepositoryPublishSkipsUnchangedTenThousandObjectHistory(t *testing.T) {
3535
3636 // Replaying the now-current generation still parses the 10k references, but
3737 // must not allocate one object-sized buffer per retained history entry.
38- // The limit is 350, not lower: the os.Root-based fast path pays a constant
39- // ~15 extra allocations (2 guarded reads, each a Root.Lstat symlink check
40- // plus a Root.OpenFile, and os.Root re-walks the path per call), and -race
41- // adds ~20 more. A per-retained-object regression would exceed this by
42- // orders of magnitude.
38+ // The limit is 450, not lower: the os.Root-based fast path pays a constant
39+ // allocation cost for guarded reads and root identity validation, and -race
40+ // adds further overhead. A per-retained-object regression would exceed this
41+ // by orders of magnitude.
4342 allocations := testing.AllocsPerRun(3, func() {
4443 if err := repo.Publish(context.Background(), second); err != nil {
4544 panic(err)
4645 }
4746 })
48- if allocations > 350 {
49- t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 350; retained objects were likely copied", allocations)
47+ if allocations > 450 {
48+ t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 450; retained objects were likely copied", allocations)
5049 }
5150
5251 got, err := repo.Load(context.Background(), second.Name)
@@ -35,18 +35,17 @@ func TestRepositoryPublishSkipsUnchangedTenThousandObjectHistory(t *testing.T) {
35 35
36 // Replaying the now-current generation still parses the 10k references, but36 // Replaying the now-current generation still parses the 10k references, but
37 // must not allocate one object-sized buffer per retained history entry.37 // must not allocate one object-sized buffer per retained history entry.
38- // The limit is 350, not lower: the os.Root-based fast path pays a constant38+ // The limit is 450, not lower: the os.Root-based fast path pays a constant
39- // ~15 extra allocations (2 guarded reads, each a Root.Lstat symlink check39+ // allocation cost for guarded reads and root identity validation, and -race
40- // plus a Root.OpenFile, and os.Root re-walks the path per call), and -race40+ // adds further overhead. A per-retained-object regression would exceed this
41- // adds ~20 more. A per-retained-object regression would exceed this by41+ // by orders of magnitude.
42- // orders of magnitude.
43 allocations := testing.AllocsPerRun(3, func() {42 allocations := testing.AllocsPerRun(3, func() {
44 if err := repo.Publish(context.Background(), second); err != nil {43 if err := repo.Publish(context.Background(), second); err != nil {
45 panic(err)44 panic(err)
46 }45 }
47 })46 })
48- if allocations > 350 {47+ if allocations > 450 {
49- t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 350; retained objects were likely copied", allocations)48+ t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 450; retained objects were likely copied", allocations)
50 }49 }
51 50
52 got, err := repo.Load(context.Background(), second.Name)51 got, err := repo.Load(context.Background(), second.Name)
M internal/adapters/snapshot/safe_path.go +25 -1
@@ -12,7 +12,31 @@ import (
1212 )
1313
1414 func (r *Repository) openRoot() (*os.Root, error) {
15- return os.OpenRoot(r.dir)
15+ // os.OpenRoot follows a final symlink, so validate that its pinned directory
16+ // is still the non-symlink directory at the configured path.
17+ root, err := os.OpenRoot(r.dir)
18+ if err != nil {
19+ return nil, err
20+ }
21+ current, err := os.Lstat(r.dir)
22+ if err != nil {
23+ return nil, closeRootOnError(root, err)
24+ }
25+ if !current.IsDir() || current.Mode()&os.ModeSymlink != 0 {
26+ return nil, closeRootOnError(root, &os.PathError{Op: "open", Path: r.dir, Err: syscall.ELOOP})
27+ }
28+ pinned, err := root.Stat(".")
29+ if err != nil {
30+ return nil, closeRootOnError(root, err)
31+ }
32+ if !os.SameFile(current, pinned) {
33+ return nil, closeRootOnError(root, &os.PathError{Op: "open", Path: r.dir, Err: syscall.ESTALE})
34+ }
35+ return root, nil
36+}
37+
38+func closeRootOnError(root *os.Root, err error) error {
39+ return errors.Join(err, root.Close())
1640 }
1741
1842 // rejectFinalSymlink preserves the pre-os.Root guarantee that the final
@@ -12,7 +12,31 @@ import (
12 )12 )
13 13
14 func (r *Repository) openRoot() (*os.Root, error) {14 func (r *Repository) openRoot() (*os.Root, error) {
15- return os.OpenRoot(r.dir)15+ // os.OpenRoot follows a final symlink, so validate that its pinned directory
16+ // is still the non-symlink directory at the configured path.
17+ root, err := os.OpenRoot(r.dir)
18+ if err != nil {
19+ return nil, err
20+ }
21+ current, err := os.Lstat(r.dir)
22+ if err != nil {
23+ return nil, closeRootOnError(root, err)
24+ }
25+ if !current.IsDir() || current.Mode()&os.ModeSymlink != 0 {
26+ return nil, closeRootOnError(root, &os.PathError{Op: "open", Path: r.dir, Err: syscall.ELOOP})
27+ }
28+ pinned, err := root.Stat(".")
29+ if err != nil {
30+ return nil, closeRootOnError(root, err)
31+ }
32+ if !os.SameFile(current, pinned) {
33+ return nil, closeRootOnError(root, &os.PathError{Op: "open", Path: r.dir, Err: syscall.ESTALE})
34+ }
35+ return root, nil
36+}
37+
38+func closeRootOnError(root *os.Root, err error) error {
39+ return errors.Join(err, root.Close())
16 }40 }
17 41
18 // rejectFinalSymlink preserves the pre-os.Root guarantee that the final42 // rejectFinalSymlink preserves the pre-os.Root guarantee that the final

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close