fix(host-kit): keep extracted directories owner-accessible by alcpereira · Pull Request #3111 · callstack/agent-device

alcpereira · 2026-10-02T11:05:54Z

Summary

An archive could declare a directory without owner read access (for example 0o300). The extractor applied that mode with recursive mkdir, so implied parents got it too, and cleanup's recursive rm then failed with EACCES:

a failed extraction reported the cleanup error instead of its own, and leaked the temp tree;
a successful one leaked the extracted tree after install.

Any caller who can get an archive extracted (uploads, path sources, allowlisted URLs) could repeat this, up to the 4 GiB expansion budget each time.

Zip and tar now share one entry-mode rule in archive-safety.ts: declared permission bits only (set-id and sticky stripped, as before), at least owner rwx for directories and rw for files. Implied parents are created with the default mode; directories get their declared mode under the process umask whether declared before or after their children, so entry order does not change permissions. An explicit mode of zero is honored as owner access only; only an absent mode (including zip entries without Unix attributes) falls back to the default.

This also adds adversarial extraction tests for guarantees that had no coverage: zip/tar symlinks and hard links, device and FIFO entries, ../absolute paths, PAX path/linkpath overrides, duplicate, case-colliding, and file/directory-colliding entries, set-id stripping, and zero modes. It's groundwork for allowing archive URLs from any public host, which follows in #3110.

6 files, all in packages/host-kit/src/internal.

Validation

Tested commit: 5d9f400aa
The unreadable-directory tests fail on origin/main (4 of 20; the unfixed bug also left the run's TMPDIR undeletable) and pass after the fix. The entry-order and zero-mode tests fail against 0f45e3b and pass after 4b1b594.
pnpm check:affected --run on 5d9f400aa: all runnable checks passed (434 files, 2,917 tests, typecheck, lint, layering, fallow, build). Replay and smoke lanes are GitHub-authoritative.
Host-side extraction only, so no device run applies.

An archive could declare a directory without owner read access (e.g. 0o300). Recursive mkdir applied that mode to implied parents too, so cleanup's recursive rm failed with EACCES: a failed extraction reported the cleanup error instead of its own and leaked the temp tree, and a successful one leaked it after install. Entry modes now come from one rule shared by zip and tar: declared permission bits only, at least owner rwx for directories and rw for files. Implied parents are created with the default mode. Adds adversarial extraction tests pinning link, special-entry, escaping-path, PAX override, duplicate, case-collision, and set-id handling, none of which were covered.

cubic-dev-ai

2 issues found across 6 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/host-kit/src/internal/archive-extraction.test.ts">

<violation number="1" location="packages/host-kit/src/internal/archive-extraction.test.ts:29">
P3: `extractFixture` creates a temp `root` in os.tmpdir() for every test and never removes it, so each run leaks many temp directories (REFUSED_ENTRIES alone adds 12). Every sibling test file in this directory cleans up its temp root (e.g. archive-extraction-tar.test.ts removes `workspace.root` in a `finally`; durable-file.test.ts uses an `afterEach`). Collect each created root and remove it in an `afterEach` so the suite stops leaking temp trees.</violation>
</file>

<file name="packages/host-kit/src/internal/archive-extraction-zip.ts">

<violation number="1" location="packages/host-kit/src/internal/archive-extraction-zip.ts:162">
P2: `extractedEntryMode` does not guarantee owner access because ZIP creation APIs mask its result with the process umask. With an owner-clearing umask such as `0o777`, entries become mode `000` and nested extraction fails; chmod entries after creation, including files, to enforce the invariant.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

cubic-dev-ai · 2026-10-02T11:14:35Z

-    mode: unixMode & 0o777 || fallback,
+    kind,
+    size: kind === 'directory' ? 0 : entry.uncompressedSize,
+    mode: extractedEntryMode(kind, unixMode),


P2: extractedEntryMode does not guarantee owner access because ZIP creation APIs mask its result with the process umask. With an owner-clearing umask such as 0o777, entries become mode 000 and nested extraction fails; chmod entries after creation, including files, to enforce the invariant.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/host-kit/src/internal/archive-extraction-zip.ts, line 162: <comment>`extractedEntryMode` does not guarantee owner access because ZIP creation APIs mask its result with the process umask. With an owner-clearing umask such as `0o777`, entries become mode `000` and nested extraction fails; chmod entries after creation, including files, to enforce the invariant.</comment> <file context> @@ -152,13 +154,12 @@ function toManifestEntry(entry: yauzl.Entry): ArchiveManifestEntry { - mode: unixMode & 0o777 || fallback, + kind, + size: kind === 'directory' ? 0 : entry.uncompressedSize, + mode: extractedEntryMode(kind, unixMode), }; } </file context>

cubic-dev-ai · 2026-10-02T11:14:36Z

+  outputRoot: string;
+  error: unknown;
+}> {
+  const root = await mkdtempForTest('agent-device-archive-extraction-');


P3: extractFixture creates a temp root in os.tmpdir() for every test and never removes it, so each run leaks many temp directories (REFUSED_ENTRIES alone adds 12). Every sibling test file in this directory cleans up its temp root (e.g. archive-extraction-tar.test.ts removes workspace.root in a finally; durable-file.test.ts uses an afterEach). Collect each created root and remove it in an afterEach so the suite stops leaking temp trees.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/host-kit/src/internal/archive-extraction.test.ts, line 29: <comment>`extractFixture` creates a temp `root` in os.tmpdir() for every test and never removes it, so each run leaks many temp directories (REFUSED_ENTRIES alone adds 12). Every sibling test file in this directory cleans up its temp root (e.g. archive-extraction-tar.test.ts removes `workspace.root` in a `finally`; durable-file.test.ts uses an `afterEach`). Collect each created root and remove it in an `afterEach` so the suite stops leaking temp trees.</comment> <file context> @@ -0,0 +1,259 @@ + outputRoot: string; + error: unknown; +}> { + const root = await mkdtempForTest('agent-device-archive-extraction-'); + const archivePath = path.join(root, `fixture.${fixture.type}`); + if (fixture.type === 'zip') await writeZipFixture(archivePath, fixture.entries); </file context>

thymikee · 2026-10-02T11:32:38Z

This PR is ready at 0f45e3b. Extracted directories now stay owner-accessible, and the change is limited to host-kit archive extraction. CI is green, with 14 checks and none failing. I know of no conflicts.

Not blocking, and you can take or leave these: (1) a tar header with an explicit mode of 0 now gets the default permissions through the || fallback in https://github.com/callstack/agent-device/blob/0f45e3b/packages/host-kit/src/internal/archive-safety.ts#L194, so the default should apply only when the mode is undefined and the zip caller should pass undefined when unixMode is 0, which gives tar 0 exactly 0o700 or 0o600; (2) the extraction tests have no fixture where a child entry comes before its explicit 0o300 directory entry, and no tar file entry with mode 0o000 asserting stat.mode & 0o600; (3) extractFixture in https://github.com/callstack/agent-device/blob/0f45e3b/packages/host-kit/src/internal/archive-extraction.test.ts#L29 never removes its temp roots, unlike the tar and zip sibling tests; (4) when an explicit directory entry follows its children, the existing-directory branch keeps the default mode, which matches the old behavior and the doc comment.

I did not run the new tests on either commit. The count of 4 tests failing before the fix comes from reading the old code. I did not check the umask interaction live, and the case-collision test accepts either outcome, so it does not tell case-sensitive and case-insensitive filesystems apart. No further step is needed before merge.

A directory entry that follows its children now takes its declared mode instead of keeping the implied default, so entry order no longer changes extracted permissions. An explicit mode of zero is honored as owner access only; only an absent mode falls back to the default, and zip entries without Unix attributes still get it. The tar fixture can now encode a zero mode, which tar-stream otherwise replaces with its default. Tests cover both entry orders, zero modes, attribute-less zip entries, and a directory colliding with an earlier file.

cubic-dev-ai

2 issues found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/host-kit/src/internal/archive-extraction.fixtures.ts">

<violation number="1" location="packages/host-kit/src/internal/archive-extraction.fixtures.ts:128">
P3: This raw 100-byte ASCII name lookup misses valid tar entry names that use the USTAR prefix or contain non-ASCII or significant spaces. The helper then fails to restore an explicit zero mode, so tests for those paths exercise the default mode instead; reconstruct the complete header name with the archive's encoding before matching.</violation>
</file>

<file name="packages/host-kit/src/internal/archive-extraction.test.ts">

<violation number="1" location="packages/host-kit/src/internal/archive-extraction.test.ts:246">
P3: This test asserts exact modes `0o700`/`0o600` for entries created fresh via `fs.mkdir(..., { mode })` and `createWriteStream(..., { mode })`, but Node masks those modes with the process umask, so the assertions hold only under the CI default umask. The sibling "zip entry without Unix attributes" test added in the same delta compensates with `& ~process.umask()`; make this test umask-independent by asserting the guarantee (owner access present) instead of the exact mode.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

cubic-dev-ai · 2026-10-02T12:06:41Z

+  if (names.size === 0) return;
+  for (let offset = 0; offset + TAR_BLOCK <= archive.length;) {
+    const block = archive.subarray(offset, offset + TAR_BLOCK);
+    const name = readTarField(block, 0, 100);


P3: This raw 100-byte ASCII name lookup misses valid tar entry names that use the USTAR prefix or contain non-ASCII or significant spaces. The helper then fails to restore an explicit zero mode, so tests for those paths exercise the default mode instead; reconstruct the complete header name with the archive's encoding before matching.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/host-kit/src/internal/archive-extraction.fixtures.ts, line 128: <comment>This raw 100-byte ASCII name lookup misses valid tar entry names that use the USTAR prefix or contain non-ASCII or significant spaces. The helper then fails to restore an explicit zero mode, so tests for those paths exercise the default mode instead; reconstruct the complete header name with the archive's encoding before matching.</comment> <file context> @@ -99,14 +99,54 @@ export async function writeTarFixture( + if (names.size === 0) return; + for (let offset = 0; offset + TAR_BLOCK <= archive.length;) { + const block = archive.subarray(offset, offset + TAR_BLOCK); + const name = readTarField(block, 0, 100); + if (!name) return; + if (names.has(name.replace(/\/$/, ''))) { </file context>

cubic-dev-ai · 2026-10-02T12:06:41Z

+  const { outputRoot, error } = await extractFixture(fixture);
+
+  assert.equal(error, undefined);
+  assert.equal((await fs.stat(path.join(outputRoot, 'App.app/private'))).mode & 0o777, 0o700);


P3: This test asserts exact modes 0o700/0o600 for entries created fresh via fs.mkdir(..., { mode }) and createWriteStream(..., { mode }), but Node masks those modes with the process umask, so the assertions hold only under the CI default umask. The sibling "zip entry without Unix attributes" test added in the same delta compensates with & ~process.umask(); make this test umask-independent by asserting the guarantee (owner access present) instead of the exact mode.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/host-kit/src/internal/archive-extraction.test.ts, line 246: <comment>This test asserts exact modes `0o700`/`0o600` for entries created fresh via `fs.mkdir(..., { mode })` and `createWriteStream(..., { mode })`, but Node masks those modes with the process umask, so the assertions hold only under the CI default umask. The sibling "zip entry without Unix attributes" test added in the same delta compensates with `& ~process.umask()`; make this test umask-independent by asserting the guarantee (owner access present) instead of the exact mode.</comment> <file context> @@ -199,6 +199,95 @@ test.each<ArchiveFixture>([ + const { outputRoot, error } = await extractFixture(fixture); + + assert.equal(error, undefined); + assert.equal((await fs.stat(path.join(outputRoot, 'App.app/private'))).mode & 0o777, 0o700); + assert.equal( + (await fs.stat(path.join(outputRoot, 'App.app/private/secret'))).mode & 0o777, </file context>

Matching headers by name would patch every duplicate when one declares a zero mode and depended on the 100-byte name field. The fixture now patches by emitted entry order, skipping the PAX extended header that precedes an entry, and the zero-mode tar test carries a PAX record to prove the skip.

thymikee · 2026-10-02T12:30:31Z

At 4b1b594 the new mode-0 handling looks right, but the new directory rule still depends on entry order. I removed the ready-for-human label until this is fixed. CI is green and there are no conflicts.

In https://github.com/callstack/agent-device/blob/4b1b594/packages/host-kit/src/internal/archive-safety.ts#L212, fs.mkdir(path, { mode }) applies the umask, but the EEXIST branch's fs.chmod(outputPath, mode) does not. So with umask 022, a directory declared 0o777 before its children extracts as 0o755, and the same directory declared after its children extracts as 0o777 (world-writable). The doc comment above says entry order does not change the permissions. outputRoot is created with a plain fs.mkdir, so whether other users can reach that directory depends on the caller's temp parent. I did not check every caller. The new test does not catch this, because the umask does not change 0o700.

Could both branches use one rule, for example chmod to (mode & ~umask) | owner access after the mkdir? Please add a test that declares a 0o777 directory after a child and expects 0o777 & ~process.umask(). This would also answer the open umask review thread. I did not run the tests locally.

The newer head 85fec93 changes only the test fixture and tests, so this still applies there.

mkdir applies the process umask to its mode and chmod does not, so a directory declared after its children extracted with more permissions than the same directory declared before them. Both branches now chmod to the declared mode under the umask, with owner access kept, matching what the kernel does for files. The entry-order test declares 0o770 and expects it under the umask in both orders.

thymikee · 2026-10-02T13:31:07Z

Looks good at 5d9f400. The directory mode rule no longer depends on entry order: new and existing directories both get (mode & ~umask) | 0o700, so cleanup can always remove the tree, and the new test declares a directory after its child. This also answers the earlier change request on the directory mode rule.

Optional: use 0o777 instead of 0o770 in the entry-order test, so it also fails on the old code under umask 002.

…lugin * origin/main: (628 commits) fix(ios): pin runner build roots under derived data (callstack#3158) feat: add managed provider plugin infrastructure (callstack#3121) fix(ios): report keyboard focus from the AX bridge's is-editing trait (callstack#3163) feat(devices): report model and osVersion (callstack#3119) fix: guard alert deadline before native tap synthesis (callstack#3113) feat(install-source): accept archive URLs from any public host (callstack#3110) test: keep uptime responsive behind busy runner work (callstack#3114) fix(apple): read the launch confirmation whenever the open cannot see the app (callstack#3115) fix(host-kit): keep extracted directories owner-accessible (callstack#3111) fix(daemon): run Apple tools with the requesting client's DEVELOPER_DIR (callstack#3109) fix(daemon): fence daemon.json removal to its owning process (callstack#3102) fix(snapshot): stop sibling-sized chrome containers from covering their own region (callstack#2996) (callstack#3097) fix(ios): stop reading windows past the one the runner resolved (callstack#3103) refactor(daemon): apply one dispatch-disclosure rule to returned and thrown failures (callstack#3099) chore: drop unused production exports and suppress dynamic consumers (callstack#3100) docs(help): document wait readiness and restart exhaustion (callstack#3098) docs: simplify Host to fresh Simlock devices and lease recovery (callstack#3095) feat(capture): report the display rotation a screenshot was rendered in (callstack#3088) refactor(snapshot): preserve normalized node attributes through presentation (callstack#3092) refactor(help): colocate fold guidance and extract workflows (callstack#3093) ... # Conflicts: # README.md # package.json # packages/kernel/src/snapshot.ts # src/__tests__/eager-closure-budgets.ts # src/cli/commands/connection-presentation.ts # src/commands/schema/cli-help.ts # src/commands/schema/command-overrides.ts

alcpereira mentioned this pull request Oct 2, 2026

feat(install-source): accept archive URLs from any public host #3110

Merged

cubic-dev-ai Bot reviewed Oct 2, 2026

View reviewed changes

thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 2, 2026

cubic-dev-ai Bot reviewed Oct 2, 2026

View reviewed changes

thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 2, 2026

thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 2, 2026

thymikee merged commit 41b3804 into callstack:main Oct 2, 2026
14 checks passed

Navigation Menu

Uh oh!

fix(host-kit): keep extracted directories owner-accessible - #3111

fix(host-kit): keep extracted directories owner-accessible#3111
thymikee merged 4 commits into
callstack:mainfrom
alcpereira:fix/archive-owner-accessible-dirs

alcpereira commented Oct 2, 2026 •

edited

Loading

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

cubic-dev-ai Bot Oct 2, 2026

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot Oct 2, 2026

Uh oh!

thymikee commented Oct 2, 2026

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

cubic-dev-ai Bot Oct 2, 2026

Uh oh!

cubic-dev-ai Bot Oct 2, 2026

Uh oh!

thymikee commented Oct 2, 2026

Uh oh!

thymikee commented Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

alcpereira commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

thymikee commented Oct 2, 2026

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

thymikee commented Oct 2, 2026

Uh oh!

thymikee commented Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alcpereira commented Oct 2, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading