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
10 changes: 8 additions & 2 deletions environment/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,11 @@ func (e *Environment) unzipSkill(data []byte, dest string) error {
}

if f.FileInfo().IsDir() {
if err := os.MkdirAll(absTarget, f.Mode()); err != nil {
// Clamp to owner-rwx: zips normalized by older fc-safari builds
// carry dir entries with no mode bits (read back as 0666 — no
// execute), and honoring that verbatim creates a directory we
// cannot extract files into (EACCES on every child).
if err := os.MkdirAll(absTarget, f.Mode().Perm()|0o700); err != nil {
return fmt.Errorf("failed to create directory %s: %w", cleanName, err)
}
continue
Expand All @@ -1211,7 +1215,9 @@ func (e *Environment) extractZipFile(f *zip.File, targetPath string) error {
_ = rc.Close()
}()

dst, err := os.OpenFile(targetPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
// Clamp to owner-rw so a mode-less zip entry can't produce a file the
// runner itself cannot re-read (same normalization gap as directories).
dst, err := os.OpenFile(targetPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode().Perm()|0o600)
if err != nil {
return err
}
Expand Down
41 changes: 41 additions & 0 deletions environment/environment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -625,3 +625,44 @@ func TestSyncSkill_BadZipKeepsExistingInstall(t *testing.T) {
assert.NotContains(t, e.Name(), ".installing-", "staging dir must be cleaned up on failure")
}
}

// TestSyncSkill_ModelessDirEntryZip reproduces the prod skill-load failure
// (sess_f7Nt..., 2026-06-11): fc-safari's zip normalization rewrote entries
// without mode bits, so an explicit directory entry reads back as 0666 — no
// execute bit. Honoring that verbatim created an un-traversable staging dir
// and every file inside failed with EACCES. The extractor must clamp to
// owner-rwx so such archives (already persisted in S3) still install.
func TestSyncSkill_ModelessDirEntryZip(t *testing.T) {
ws := newTestEnvironment(t)

var buf bytes.Buffer
w := zip.NewWriter(&buf)
// CreateHeader without SetMode — exactly what the buggy normalizer emitted.
_, err := w.CreateHeader(&zip.FileHeader{Name: "references/"})
require.NoError(t, err)
f, err := w.CreateHeader(&zip.FileHeader{Name: "references/worked-example.md", Method: zip.Deflate})
require.NoError(t, err)
_, err = f.Write([]byte("example"))
require.NoError(t, err)
f, err = w.CreateHeader(&zip.FileHeader{Name: "SKILL.md", Method: zip.Deflate})
require.NoError(t, err)
_, err = f.Write([]byte("# skill"))
require.NoError(t, err)
require.NoError(t, w.Close())

res, err := ws.SyncSkill(context.Background(), &protocol.SyncSkillArgs{
SkillName: "modeless",
Checksum: "csum-modeless",
ZipData: base64.StdEncoding.EncodeToString(buf.Bytes()),
})
require.NoError(t, err)
require.True(t, res.Success)

dir := filepath.Join(ws.Root(), "skills", "modeless")
info, err := os.Stat(filepath.Join(dir, "references"))
require.NoError(t, err)
assert.NotZero(t, info.Mode().Perm()&0o700, "staging dirs must stay owner-traversable")
body, err := os.ReadFile(filepath.Join(dir, "references", "worked-example.md"))
require.NoError(t, err)
assert.Equal(t, "example", string(body))
}
Loading