Skip to content

fix(environment): clamp zip entry modes during skill extraction#68

Merged
ysyneu merged 1 commit into
feat/ai-srefrom
fix/skill-unzip-dirmode-clamp
Jun 15, 2026
Merged

fix(environment): clamp zip entry modes during skill extraction#68
ysyneu merged 1 commit into
feat/ai-srefrom
fix/skill-unzip-dirmode-clamp

Conversation

@ysyneu

@ysyneu ysyneu commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Problem

Prod skill loads fail deterministically with:

failed to unzip skill: failed to extract references/worked-example.md:
open .../skills/.installing-<skill>/references/worked-example.md: permission denied

Root cause (reproduced byte-for-byte): fc-safari's zip normalization rewrote entries without mode bits. A mode-less directory entry reads back as 0666 — no execute bit — and unzipSkill honored it via MkdirAll(absTarget, f.Mode()), creating an un-traversable staging dir. Every file inside then fails with EACCES.

Fix

Clamp extraction modes: dirs Perm()|0o700, files Perm()|0o600. OR only adds bits, so archives with sane modes (incl. exec bits on scripts) are unaffected. The runner-side clamp is required even though fc-safari is fixed separately — broken normalized zips are already persisted in S3.

Verification

  • New regression test TestSyncSkill_ModelessDirEntryZip: red on old code with the exact prod error, green with fix.
  • Standalone harness calling real environment.New + SyncSkill on a prod-replica zip: full install chain (staging → checksum → rename) succeeds, file readable, dir traversable.
  • Cross-repo E2E: zip produced by the patched fc-safari normalizer installs cleanly, script exec bit survives end-to-end.
  • go build ./... && go vet ./... && go test ./... all green; gofumpt clean.

Skill zips normalized by older fc-safari builds carry entries with no mode
bits: a directory entry reads back as 0666 (no execute bit), and honoring it
verbatim creates an un-traversable staging dir — every file inside then fails
with EACCES (prod skill-load failures, 2026-06-11). Clamp dirs to owner-rwx
and files to owner-rw so such archives still install; archives with sane
modes are unaffected.
@ysyneu ysyneu changed the base branch from main to feat/ai-sre June 11, 2026 13:45
@ysyneu ysyneu merged commit 4291ded into feat/ai-sre Jun 15, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant