Skip to content

fix(fs): address an app container by bundle id and a relative path - #484

Open
mobile-kevin wants to merge 1 commit into
mobile-next:mainfrom
mobile-kevin:fix-app-container-fs
Open

mobile-kevin wants to merge 1 commit into
mobile-next:mainfrom
mobile-kevin:fix-app-container-fs

Conversation

@mobile-kevin

Copy link
Copy Markdown
Collaborator

Problem

The documented per-container form fs ls/mkdir/rm <bundle-id> <path> (help example: fs ls com.example.app /Documents) worked on neither platform:

  • Android ignored the bundle id and used the path as-is, so <bundle> /files ran ls /files at the device root and failed (ls failed: exit status 1), and <bundle> with no path listed the device root instead of the container.
  • iOS simulator used the bundle id only when no path was given; with a path it validated the raw path and rejected it ('/Documents' is outside the simulator device directory).

So the only way to reach a container was the full absolute path, which the [bundle-id] argument is meant to save you from.

Fix

Resolve (bundle, path) to the container before doing anything:

  • Android builds /data/user/0/<pkg>/<path>, which makes the existing buildShellCommand wrap the op in run-as <pkg>.
  • The simulator joins the path under the app's data container, cleaning the relative part (filepath.Clean("/"+path)) so a .. can't climb out; validatePath still guards the result.
  • An empty path lists the container root on both platforms (previously the device root on Android).

fs push/fs pull keep their <local> <remote> signatures (no bundle slot), so container writes still use a full path; this PR fixes the ls/mkdir/rm forms that take a bundle id.

Tests

  • Unit: androidContainerPath builds the right /data/user/0/... path and routes through run-as.
  • E2E (android + simulator): <bundle> <relpath> lists the same as the absolute path, <bundle> lists the container root (not the device root), create/remove through the bundle form works, a ../ relative path is rejected, and an unknown package errors.
  • Verified live on the emulator and the iPhone simulator.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 5 billable files and costs up to $1.25.

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 5a38de2 and 278de08.

Files selected for processing: 5

  • devices/android_fs.go
  • devices/android_fs_container_test.go
  • devices/simulator_fs.go
  • test/android.spec.ts
  • test/simulator.spec.ts

@gmegidish
gmegidish force-pushed the fix-app-container-fs branch from 8462a5b to 212c474 Compare November 13, 2026 10:48
`fs ls/mkdir/rm <bundle-id> <path>` (the documented form, e.g.
`fs ls com.example.app /Documents`) did not work on either platform:

- Android ignored the bundle id and ran the path as-is, so `<bundle> /files`
  became `ls /files` at the device root and failed, and `<bundle>` with no path
  listed the device root instead of the container.
- The iOS simulator only used the bundle id when no path was given; with a path
  it validated the raw path and rejected it as outside the device directory.

Resolve (bundle, path) to the container up front: Android builds
/data/user/0/<pkg>/<path> (which routes through run-as), and the simulator joins
the path under the app's data container, cleaning the relative part so a ".."
cannot climb out. An empty path lists the container root on both.

Adds unit tests for the Android path builder and e2e tests on both platforms for
the bundle+relative form (list/create/remove and a rejected "../" escape).
@gmegidish
gmegidish force-pushed the fix-app-container-fs branch from 212c474 to 278de08 Compare November 13, 2026 11:57
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