Skip to content

Commit dbb5cd0

Browse files
smoserclaude
andcommitted
build: keep setuid/setgid/sticky on unpacked directories
retrieveWorkspace created directories with MkdirAll(hdr.Name, mode.Perm()). .Perm() masks to 0o777 and nothing applied the special bits afterwards, so a directory the guest workspace has as 1777 or 2755 landed on the host as 0777 or 0755 and those bits never reached the built apk. wolfi-baselayout chmods /tmp and /var/tmp to 1777, and the published wolfi-baselayout apk records both as 0777. Chmod the full mode after MkdirAll when the header carries any of setuid/setgid/sticky. Directories without those bits are left alone, so the melange-out directories melange pre-creates keep their current handling. The regular file branch already passes the full mode to OpenFile, so it needs no change. The bubblewrap OCI layer extraction had the same shape, which left the build environment's own /tmp non-sticky. os.MkdirAll there also applies the umask, so plain 0777 was lost as well; passing mode.Perm()|special to os.Chmod restores both. Only the qemu runner retrieves a workspace tar -- bubblewrap and docker bind-mount the workspace, where WorkspaceTar is a no-op. Fixes #2642 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7e518c0 commit dbb5cd0

3 files changed

Lines changed: 148 additions & 2 deletions

File tree

pkg/build/build.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1100,6 +1100,10 @@ func isValidPath(targetPath, baseDir string) error {
11001100
return nil
11011101
}
11021102

1103+
// specialModeBits are the mode bits that Mkdir/MkdirAll cannot set, since
1104+
// they take only permission bits.
1105+
const specialModeBits = os.ModeSetuid | os.ModeSetgid | os.ModeSticky
1106+
11031107
// retrieveWorkspace retrieves the workspace from the container and unpacks it
11041108
// to the workspace directory. The workspace retrieved from the runner is in a
11051109
// tar stream containing the workspace contents rooted at ./melange-out
@@ -1159,10 +1163,19 @@ func (b *Build) retrieveWorkspace(ctx context.Context, fs apkofs.FullFS) error {
11591163
}
11601164
}
11611165

1162-
if err := fs.MkdirAll(hdr.Name, hdr.FileInfo().Mode().Perm()); err != nil {
1166+
mode := hdr.FileInfo().Mode()
1167+
if err := fs.MkdirAll(hdr.Name, mode.Perm()); err != nil {
11631168
return fmt.Errorf("unable to create directory %s: %w", hdr.Name, err)
11641169
}
11651170

1171+
// MkdirAll carries only permission bits, so setuid/setgid/sticky
1172+
// have to be applied separately.
1173+
if special := mode & specialModeBits; special != 0 {
1174+
if err := fs.Chmod(hdr.Name, mode.Perm()|special); err != nil {
1175+
return fmt.Errorf("unable to chmod directory %s: %w", hdr.Name, err)
1176+
}
1177+
}
1178+
11661179
if err := fs.Chown(hdr.Name, uid, gid); err != nil {
11671180
return fmt.Errorf("unable to chown directory %s: %w", hdr.Name, err)
11681181
}

pkg/build/workspace_test.go

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// Copyright 2026 Chainguard, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package build
16+
17+
import (
18+
"archive/tar"
19+
"bytes"
20+
"context"
21+
"io"
22+
"os"
23+
"testing"
24+
25+
apkofs "chainguard.dev/apko/pkg/apk/fs"
26+
apko_build "chainguard.dev/apko/pkg/build"
27+
28+
"chainguard.dev/melange/pkg/config"
29+
"chainguard.dev/melange/pkg/container"
30+
)
31+
32+
// tarRunner is a container.Runner that only implements WorkspaceTar, returning
33+
// a canned tar stream.
34+
type tarRunner struct {
35+
tarball []byte
36+
}
37+
38+
func (t *tarRunner) WorkspaceTar(context.Context, *container.Config, []string) (io.ReadCloser, error) {
39+
return io.NopCloser(bytes.NewReader(t.tarball)), nil
40+
}
41+
42+
func (t *tarRunner) Close() error { return nil }
43+
func (t *tarRunner) Name() string { return "tar-runner" }
44+
func (t *tarRunner) TestUsability(context.Context) bool { return true }
45+
func (t *tarRunner) OCIImageLoader() container.Loader { return nil }
46+
func (t *tarRunner) StartPod(context.Context, *container.Config) error { return nil }
47+
func (t *tarRunner) Run(context.Context, *container.Config, map[string]string, ...string) error {
48+
return nil
49+
}
50+
func (t *tarRunner) TerminatePod(context.Context, *container.Config) error { return nil }
51+
func (t *tarRunner) TempDir() string { return "" }
52+
func (t *tarRunner) GetReleaseData(context.Context, *container.Config) (*apko_build.ReleaseData, error) {
53+
return nil, nil
54+
}
55+
56+
var _ container.Runner = (*tarRunner)(nil)
57+
58+
// TestRetrieveWorkspaceSpecialModeBits verifies that setuid, setgid and sticky
59+
// bits survive the trip through retrieveWorkspace. Regression test for
60+
// https://github.com/chainguard-dev/melange/issues/2642, where directories
61+
// were created with Mode().Perm() and nothing set the special bits after.
62+
func TestRetrieveWorkspaceSpecialModeBits(t *testing.T) {
63+
entries := []struct {
64+
name string
65+
typeflag byte
66+
mode int64
67+
}{
68+
{"melange-out/pkg/", tar.TypeDir, 0o755},
69+
{"melange-out/pkg/tmp/", tar.TypeDir, 0o1777},
70+
{"melange-out/pkg/var/", tar.TypeDir, 0o755},
71+
{"melange-out/pkg/var/tmp/", tar.TypeDir, 0o1777},
72+
{"melange-out/pkg/usr/", tar.TypeDir, 0o755},
73+
{"melange-out/pkg/usr/mail/", tar.TypeDir, 0o2755},
74+
{"melange-out/pkg/usr/setuid-dir/", tar.TypeDir, 0o4755},
75+
{"melange-out/pkg/usr/postdrop", tar.TypeReg, 0o2755},
76+
}
77+
78+
var buf bytes.Buffer
79+
tw := tar.NewWriter(&buf)
80+
for _, e := range entries {
81+
if err := tw.WriteHeader(&tar.Header{
82+
Name: e.name,
83+
Typeflag: e.typeflag,
84+
Mode: e.mode,
85+
}); err != nil {
86+
t.Fatalf("writing header %s: %v", e.name, err)
87+
}
88+
}
89+
if err := tw.Close(); err != nil {
90+
t.Fatalf("closing tar: %v", err)
91+
}
92+
93+
ctx := context.Background()
94+
workspaceDir := t.TempDir()
95+
b := &Build{
96+
Configuration: &config.Configuration{},
97+
WorkspaceDir: workspaceDir,
98+
Runner: &tarRunner{tarball: buf.Bytes()},
99+
}
100+
fsys := apkofs.DirFS(ctx, workspaceDir)
101+
102+
if err := b.retrieveWorkspace(ctx, fsys); err != nil {
103+
t.Fatalf("retrieveWorkspace: %v", err)
104+
}
105+
106+
for _, e := range entries {
107+
// Let archive/tar do the mode conversion, so the expectation is in
108+
// terms of os.ModeSetuid/Setgid/Sticky rather than raw octal.
109+
hdr := tar.Header{Name: e.name, Typeflag: e.typeflag, Mode: e.mode}
110+
want := hdr.FileInfo().Mode() & (os.ModePerm | specialModeBits)
111+
112+
fi, err := fsys.Stat(e.name)
113+
if err != nil {
114+
t.Errorf("stat %s: %v", e.name, err)
115+
continue
116+
}
117+
if got := fi.Mode() & (os.ModePerm | specialModeBits); got != want {
118+
t.Errorf("%s: got mode %v (%04o), want %v (%04o)", e.name, got, got.Perm(), want, want.Perm())
119+
}
120+
}
121+
}

pkg/container/bubblewrap_runner.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ var _ Debugger = (*bubblewrap)(nil)
4242
const (
4343
BubblewrapName = "bubblewrap"
4444
buildUserID = "1000"
45+
46+
// specialModeBits are the mode bits that Mkdir/MkdirAll cannot set,
47+
// since they take only permission bits.
48+
specialModeBits = os.ModeSetuid | os.ModeSetgid | os.ModeSticky
4549
)
4650

4751
// defaultCapabilities is the set of Linux capabilities granted by the
@@ -325,9 +329,17 @@ func (b *bubblewrapOCILoader) LoadImage(ctx context.Context, layer v1.Layer, arc
325329
}
326330
switch hdr.Typeflag {
327331
case tar.TypeDir:
328-
if err := os.MkdirAll(fullname, hdr.FileInfo().Mode().Perm()); err != nil {
332+
mode := hdr.FileInfo().Mode()
333+
if err := os.MkdirAll(fullname, mode.Perm()); err != nil {
329334
return ref, fmt.Errorf("failed to create directory %s: %w", fullname, err)
330335
}
336+
// MkdirAll carries only permission bits (and applies the umask),
337+
// so setuid/setgid/sticky have to be applied separately.
338+
if special := mode & specialModeBits; special != 0 {
339+
if err := os.Chmod(fullname, mode.Perm()|special); err != nil {
340+
return ref, fmt.Errorf("failed to chmod directory %s: %w", fullname, err)
341+
}
342+
}
331343
continue
332344
case tar.TypeReg:
333345
f, err := os.OpenFile(fullname, os.O_CREATE|os.O_WRONLY, hdr.FileInfo().Mode().Perm()) // #nosec G304 - Extracting OCI image layer file

0 commit comments

Comments
 (0)