Skip to content

Commit ab87b71

Browse files
committed
refactor: replace hardcoded pixel UID/GID literals with sandbox/user package constants
1 parent 9d454c2 commit ab87b71

4 files changed

Lines changed: 20 additions & 19 deletions

File tree

internal/mcp/tools.go

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
"github.com/deevus/pixels/internal/config"
2020
"github.com/deevus/pixels/sandbox"
21+
"github.com/deevus/pixels/sandbox/user"
2122
)
2223

2324
// Tools is the dependency bundle every MCP handler closes over.
@@ -190,17 +191,6 @@ func (t *Tools) requireSandbox(name string) (Sandbox, error) {
190191
return sb, nil
191192
}
192193

193-
// pixelUID/pixelGID is the unprivileged sandbox user that MCP file operations
194-
// write as. Files written by the MCP write_file tool are chowned to this
195-
// owner so subsequent exec calls (which also run as this user) can read and
196-
// modify them. Hardcoded to match the convention established by the
197-
// provisioning scripts (rc.local creates `pixel` at uid 1000); MCP cannot
198-
// write root-owned files by design.
199-
const (
200-
pixelUID = 1000
201-
pixelGID = 1000
202-
)
203-
204194
// editFileMaxBytes caps the in-memory read for EditFile. Editing past this
205195
// would silently truncate the rest of the file on write-back.
206196
const editFileMaxBytes = 10 * 1024 * 1024
@@ -628,7 +618,7 @@ func (t *Tools) WriteFile(ctx context.Context, in WriteFileIn) (WriteFileOut, er
628618
}
629619
defer t.Locks.Acquire(sb.Name)()
630620

631-
if err := t.Backend.WriteFile(ctx, sb.Name, in.Path, []byte(in.Content), mode, pixelUID, pixelGID); err != nil {
621+
if err := t.Backend.WriteFile(ctx, sb.Name, in.Path, []byte(in.Content), mode, user.UID, user.GID); err != nil {
632622
return WriteFileOut{}, err
633623
}
634624
t.touch(sb.Name)
@@ -710,7 +700,7 @@ func (t *Tools) EditFile(ctx context.Context, in EditFileIn) (EditFileOut, error
710700
updated = strings.Replace(original, in.OldString, in.NewString, 1)
711701
}
712702

713-
if err := t.Backend.WriteFile(ctx, sb.Name, in.Path, []byte(updated), 0o644, pixelUID, pixelGID); err != nil {
703+
if err := t.Backend.WriteFile(ctx, sb.Name, in.Path, []byte(updated), 0o644, user.UID, user.GID); err != nil {
714704
return EditFileOut{}, fmt.Errorf("write: %w", err)
715705
}
716706
t.touch(sb.Name)

internal/mcp/tools_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"github.com/deevus/pixels/internal/config"
1616
"github.com/deevus/pixels/sandbox"
17+
"github.com/deevus/pixels/sandbox/user"
1718
)
1819

1920
type cloneRecord struct {
@@ -926,7 +927,7 @@ func TestWriteFileChownsToPixel(t *testing.T) {
926927
if !ok {
927928
t.Fatal("WriteFile must request explicit ownership for MCP tools")
928929
}
929-
if want := [2]int{pixelUID, pixelGID}; got != want {
930+
if want := [2]int{user.UID, user.GID}; got != want {
930931
t.Errorf("owner = %v, want %v (MCP must always write as the pixel user)", got, want)
931932
}
932933
}
@@ -948,7 +949,7 @@ func TestEditFileChownsToPixel(t *testing.T) {
948949
if !ok {
949950
t.Fatal("EditFile must request explicit ownership when writing back")
950951
}
951-
if want := [2]int{pixelUID, pixelGID}; got != want {
952+
if want := [2]int{user.UID, user.GID}; got != want {
952953
t.Errorf("owner = %v, want %v", got, want)
953954
}
954955
}

sandbox/incus/config.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"path/filepath"
77
"strconv"
88
"strings"
9+
10+
"github.com/deevus/pixels/sandbox/user"
911
)
1012

1113
// incusCfg holds parsed backend configuration.
@@ -51,8 +53,8 @@ func parseCfg(m map[string]string) (*incusCfg, error) {
5153
pool: "default",
5254
sshUser: "pixel",
5355
sshKey: "~/.ssh/id_ed25519",
54-
uid: 1000,
55-
gid: 1000,
56+
uid: user.UID,
57+
gid: user.GID,
5658
provision: true,
5759
devtools: true,
5860
egress: "unrestricted",

sandbox/truenas/client_test.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ func TestProvision(t *testing.T) {
237237
DevTools: true,
238238
},
239239
pool: "tank",
240-
wantCalls: 8, // dns + sshd config + profile.d + env + root key + pixel key + setup script + rc.local
240+
wantCalls: 9, // dns + sshd config + profile.d + env + root key + pixel key + mise.toml + setup script + rc.local
241241
check: func(t *testing.T, calls []fsWriteCall) {
242242
paths := make(map[string]fsWriteCall)
243243
for _, c := range calls {
@@ -256,12 +256,20 @@ func TestProvision(t *testing.T) {
256256
if setup.mode != 0o755 {
257257
t.Errorf("setup script mode = %o, want 755", setup.mode)
258258
}
259-
for _, want := range []string{"mise", "claude-code", "opencode", "codex", "su - pixel"} {
259+
for _, want := range []string{"mise install", "su - pixel"} {
260260
if !strings.Contains(setup.content, want) {
261261
t.Errorf("setup script missing %q", want)
262262
}
263263
}
264264

265+
// mise.toml lists the tools mise install will pull in.
266+
mise := paths[rootfs+"/home/pixel/.config/mise/config.toml"]
267+
for _, want := range []string{"claude-code", "opencode", "codex"} {
268+
if !strings.Contains(mise.content, want) {
269+
t.Errorf("mise.toml missing %q", want)
270+
}
271+
}
272+
265273
// No systemd unit should be written.
266274
if _, ok := paths[rootfs+"/etc/systemd/system/pixels-devtools.service"]; ok {
267275
t.Error("systemd unit should not be written (zmx handles devtools)")

0 commit comments

Comments
 (0)