Add first-party Apple mobile MCP integration - #251
Conversation
🤖 OS review · request changes · confidence 2/5Safe once the release authorization and commit-binding issues below are fixed. The integration adds the intended build/release MCP surfaces, but the release allowlist can be bypassed by renaming the connection, and execution can sign or upload code that no longer matches the approved commit.
🔁 Not merge-ready and no live session owns this branch — add the |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![open-session-os-tella-dev[bot]](https://avatars.githubusercontent.com/u/65127121?s=60&v=4){kind=small}
|
|
||
| /** Credentialed first-party release tools must never become fleet-wide. */ | ||
| export function requiresAllowedUsers(name: string): boolean { | ||
| return name.toLowerCase() === "apple-release"; |
There was a problem hiding this comment.
🔴 P1 — Release authorization is bypassed by renaming the MCP connection
The restriction identifies the credentialed capability solely by its operator-selected name. A valid POST /api/connections/mcp can add {name: "ios-release", command: "opensession", args: ["apple-mobile-mcp", "--mode", "release"], env: {...}} without allowedUsers; requiresAllowedUsers returns false, and filterMcpServers consequently exposes those credentialed release tools to every interactive session. Enforce this from the entry's command/arguments rather than its name, including the package binary entry point, and apply the same fail-closed check when resolving runtime configuration so hand-edited configs cannot bypass it.
| planId: string, | ||
| confirmation: string, | ||
| ) { | ||
| const { plan, config } = await loadPlan(projectDir, planId); |
There was a problem hiding this comment.
🔴 P1 — The approved commit is checked before, not during, the release build
loadPlan verifies the worktree commit and cleanliness once, then executePlan archives directly from that mutable worktree without another source check. For example, an IDE or concurrent session can edit Sources/App.swift after line 57 returns but before xcodebuild starts at line 93; the modified source is then signed and potentially uploaded while the result still reports the approved plan.commit. That breaks the advertised commit-bound approval guarantee. Materialize the planned commit in a private immutable checkout and build from it, rather than executing the signed commands against the caller's live worktree.
| const realParent = realpathSync(parent); | ||
| if (!isWithin(projectDir, realParent)) | ||
| throw new Error(`Output parent escapes project: ${input}`); | ||
| return unresolved; |
There was a problem hiding this comment.
🟠 P2 — Existing output-directory symlinks escape the project boundary
The mustExist: false path validates only the output path's parent. If .build/apple-mobile already exists as a symlink to /Users/operator/protected, that parent is the in-project .build, so the helper returns the unresolved symlink and plan/output creation writes outside the allowed project. This is the exact mode used for release.artifactDirectory. Resolve and boundary-check unresolved itself when it already exists; only use the nearest-parent logic for paths that do not yet exist.
| return unresolved; | |
| if (existsSync(unresolved)) { | |
| const candidate = realpathSync(unresolved); | |
| if (!isWithin(projectDir, candidate)) | |
| throw new Error(`Output path escapes project: ${input}`); | |
| return candidate; | |
| } | |
| return unresolved; |
|
Superseded by #252, which adds the requested Settings → Integrations setup flow on top of the first-party integration. |
1 participant
Summary
@tellahq/opensession-apple-mobileworkspace package and stableopensession apple-mobile-mcpstdio entry pointallowedUsersprotection forapple-release, project-root boundaries, protected credential paths, and authenticated commit-bound release plansVerification
bun test packages/integrations/apple-mobile packages/core/opensession-server/src/server/connections-security.test.ts packages/core/opensession-server/src/server/plugins.test.ts packages/core/opensession-server/src/server/routes/connections.test.ts(50 pass)bun run typecheckbun run lintbun run format:checkbun scripts/check-module-side-effects.tsCreated by this Assistant session