Skip to content

fix(workspace-fs): allow saving files outside the workspace root - #7982

Merged
saddlepaddle merged 1 commit into
mainfrom
saddlepaddle/fix-ui-edit-outside-repo-a55e15ca
Sep 30, 2026
Merged

saddlepaddle merged 1 commit into
mainfrom
saddlepaddle/fix-ui-edit-outside-repo-a55e15ca

Conversation

@saddlepaddle

@saddlepaddle saddlepaddle commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Saving a file from the UI failed when the file was outside the workspace root, even though the editor can open it since fix(workspace-fs): allow viewing symlinks outside workspace root #7897.
  • writeFile in packages/workspace-fs/src/fs.ts no longer checks root containment. It keeps the atomic write, writes through symlinks, keeps the file mode, and keeps the ifMatch conflict check.
  • Create directory, create unique entry, delete, move and copy stay confined to the root.

Why this is safe

The host-service caller can already read any host file and run any command in a terminal, so write confinement was not a real boundary. Related history: #7828 (reverted in #7834), #7897.

Tests

  • Flipped 2 unit tests (fs.test.ts) and 2 host-service bug-hunt integration tests from "rejects" to "writes".
  • workspace-fs: 45 pass; host-service bug-hunt suites: 35 pass; typecheck clean in both packages.
  • Not verified by saving in the running desktop app.

Summary by cubic

Allows saving files that live outside the workspace root, or through an in-root symlink pointing outside, which the UI editor previously rejected with "Path is outside workspace root". writeFile in packages/workspace-fs now follows the read policy: the same caller can already read any host file and run terminal commands, so write confinement was not a real boundary.

  • Removes the root-containment check from writeFile; create, delete, move, and copy stay confined to the root.
  • Keeps atomic writes, symlink resolution, file mode preservation, and the ifMatch conflict check.
  • Updates unit and integration tests to expect writes instead of rejections.

Written for commit 29fa64d. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Behavior Changes
    • File writes can now target paths outside the workspace, including paths reached through directory symlinks. Existing write options and revision checks remain available.
    • Structural changes, such as creating or removing filesystem entries, remain confined to the workspace.

The editor can open any file on the host since #7897, but saving one outside
the workspace root (or through an in-root symlink that resolves outside) threw
"Path is outside workspace root". writeFile now follows the read policy: the
same caller can already read any file and run any command in a terminal, so
confining edits was not a real boundary. Create, delete, move and copy stay
confined to the root.
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 1d3d27c2-9dad-4b0e-8fc9-1bca4905ec12

📥 Commits

Reviewing files that changed from the base of the PR and between 64b26a9 and 29fa64d.

📒 Files selected for processing (5)
  • packages/host-service/test/integration/bug-hunt-2.integration.test.ts
  • packages/host-service/test/integration/bug-hunt.integration.test.ts
  • packages/workspace-fs/src/fs.test.ts
  • packages/workspace-fs/src/fs.ts
  • packages/workspace-fs/src/host/service.ts
💤 Files with no reviewable changes (1)
  • packages/workspace-fs/src/host/service.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

File writes no longer enforce workspace-root containment. The host service no longer passes the root path to the write helper. Unit and integration tests now verify writes to paths outside the workspace, including through symlinks.

Changes

Host-wide file writes

Layer / File(s) Summary
Write API and host-service delegation
packages/workspace-fs/src/fs.ts, packages/workspace-fs/src/host/service.ts
writeFile no longer accepts rootPath or checks target containment and realpath. writeAtomically no longer checks the temporary file’s parent against the root. The host service omits rootPath when calling writeFile.
Workspace filesystem write tests
packages/workspace-fs/src/fs.test.ts
Tests verify writes outside the workspace and through symlinks. Tests also retain coverage for write options, revision preconditions, concurrent writes, and binary content without passing rootPath.
Host-service integration tests
packages/host-service/test/integration/bug-hunt*.integration.test.ts
Integration tests verify writes to sibling paths and outside-pointing symlink targets. The suite description states that reads and file edits are host-wide, while structural mutations remain workspace-confined.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Suggested reviewers: kitenite

Merge Risk: ⚪ Minimal · up to 29fa6

Saving outside the workspace, including through symlinks, now matches the intended host-wide editing policy. No actionable merge-blocking issue remains; desktop-app saving has not been verified.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 29fa6

Existing callers already have terminal command access, which limits the additional authority granted by host-wide saves. However, saving a private host file can temporarily expose its contents to another local user when directory permissions and default file permissions allow access. Deployment-specific protections were not established.

Retained concerns

  • Medium · security · inferred: Host-wide saves extend an existing temporary-file confidentiality weakness to private host files. Content is written before the original restrictive mode is applied. With a permissive inherited umask and an accessible parent directory, another local user can read that content before chmod; interruption can leave the temporary copy behind. Workspace containment previously rejected these outside targets.
Security review details

Security Blast Radius

  • inferred — Direct write exposure now encompasses host paths reachable under the process's effective filesystem permissions, not just one workspace. The temporary-copy concern additionally exposes saved content to local readers where directory accessibility and creation permissions permit it. This does not establish cross-host access or an operating-system privilege escalation.

Security Findings and Attack Paths

  • inferred — A legitimate save of an outside private file creates a sibling temporary copy using default creation permissions before restoring the target mode. An independently acting local reader can discover and read that copy if the directory is accessible and the inherited umask permits access. UUID naming does not prevent directory enumeration. Final chmod and cleanup reduce the window, but process termination before either completes can leave exposed content. This is a broadened exposure of an existing sequence, not a verified exploit in a production environment.

Trust Boundaries and Controls

  • inferred — The inspected callers already expose command execution under the same authentication or desktop procedure boundary, providing strong counterevidence to a new caller-authority escalation. Host-wide reads alone would not establish write authority; the terminal command paths are the decisive counterevidence. This conclusion is bounded to the inspected application paths and does not establish external gateway policies.

Hardening Proposals

  • proposed — Create temporary files exclusively with restrictive permissions before writing any content, and apply the intended final mode before rename. This would protect private content during normal writes, partial failures, and interruption rather than relying on a later chmod or inherited umask.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: allowing files outside the workspace root to be saved. It uses the required conventional commit format.
Description check ✅ Passed The description explains what changed, why it changed, safety considerations, preserved behavior, test results, and one unverified scenario. It provides equivalent Summary, Why, and Tests sections, al…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

@saddlepaddle
saddlepaddle merged commit 6130042 into main Sep 30, 2026
23 of 24 checks passed
@saddlepaddle
saddlepaddle deleted the saddlepaddle/fix-ui-edit-outside-repo-a55e15ca branch September 30, 2026 15:52
Kitenite added a commit that referenced this pull request Oct 1, 2026
* docs: weekly update for PRs merged 2026-09-23 to 2026-09-30

Source of truth: live Notion SOP "Weekly Docs Update (automation SOP)".
Window: merged:>=2026-09-23 (139 PRs). PRs through the morning of 09-23
were covered by #7771.

Changes
- Pages: no auto-open on publish (#7790); page links and URL links split
  into their own settings (#7916, #7962, #7946); Listening agents menu with
  per-row stop and Add listening agent (#7724, #7820, #7919); globe on the
  Share button (#7766); Report on public pages (#7886); agents offer a page
  (#7759). Files: pages.mdx, cli/cli-reference.mdx.
- Settings moves: Links replaced by Browser and Files & Editor (#7916,
  #7962). Files: editor.mdx, terminal-integration.mdx, customization.mdx.
- Editor: inline rename and drag to move (#7692), Auto Save (#7693, #7916),
  open and save files outside the workspace (#7897, #7982), Copy as Markdown
  (#7591), per-pane error screen (#7983). File: editor.mdx.
- Terminal copy follows Ghostty's whitespace rules (#7868).
  File: terminal-integration.mdx.
- Agents: UFO (#7898); GPT-6 Astra, Sonnet 5.5, retiring GPT-5.5, retired
  GPT-5.4/5.3 Codex (#7783, #7894, #7935). Files: agent-integration.mdx,
  cli/cli-reference.mdx.
- Host: superset start --daemon --auto-update and update-lock permission
  errors (#7735, #7952). Files: cli/host-server.mdx, cli/cli-reference.mdx.
- Removed the mcp CLI section and the Connected Service Plugins section:
  #7776 made plugins/mcp internal, the Plugins page is staff-flagged, and
  apps/docs/AGENTS.md keeps internal commands out. Files: skills.mdx,
  cli/cli-reference.mdx.
- Slack page unfurls and details panel (#7817, #7821, #7914, #7957).
  File: use-with-slack.mdx.
- Workspaces: instant creation with background naming (#7518); recoverable
  project deletion (#7896). File: workspaces.mdx.
- Shortcuts: prev/next workspace includes Pinned and wraps (#7851); ⌘B
  collapses the sidebar to a rail (#7964). File: keyboard-shortcuts.mdx.
- Ship controls and open-in button moved to the right sidebar strip (#7958).
  Files: diff-viewer.mdx, use-with-ide.mdx.

Deliberately skipped
- Cloud workspaces/environments/sandboxes (flag-gated, AGENTS.md rule):
  #7784 #7795 #7824 #7840 #7844 #7845 #7847 #7849 #7850 #7858 #7861 #7863
  #7874 #7911 #7913 #7918 #7922 #7931 #7932 #7937 #7956 #7961 #7969 #7981
  #7637 #7794 #7789 #7838 (its CLI docs already shipped in the PR).
- Staff-only plugins: #7912 Sentry plugin, #7716, #7727, #7955.
- Already documented in their own PRs: #7917, #7921.
- Removed feature never documented: #7796 Google Calendar triggers.
- Agents chip remove/revert/default-off, never documented: #7870 #7871 #7872.
- Billing/backend policy with no docs surface: #7947, #7812, #7786, #7792,
  #7798, #7761, #7948, #7949, #7951, #7827.
- Reverted security trio: #7828 #7829 #7830 (reverted by #7834).
- Fixes restoring expected behavior: #7683 #7696 #7707 #7743 #7751 #7756
  #7764 #7765 #7781 #7788 #7791 #7823 #7825 #7833 #7837 #7852 #7857 #7860
  #7864 #7865 #7869 #7873 #7897-covered #7903 #7938 #7980 #7943 #7964-chrome.
- Mobile (no docs page): #7746 #7747 #7748 #7749 #7750 #7782 #7793 #7810
  #7814 #7816 #7889 #7968.
- Marketing, internal docs, CI, version bumps: #7651 #7744 #7752 #7771 #7774
  #7776-infra #7777 #7785 #7811 #7818 #7822 #7826 #7834 #7884 #7885 #7895
  #7909 #7933 #7934 #7942 #7944 #7965 #7966.

Validation: bun run --cwd apps/docs typecheck, bun run lint:fix,
bun run lint (exit 0), Vale on 13 changed files (0 errors, 0 warnings),
git diff --check.

* chore: log weekly docs update follow-ups

* docs: tighten review findings in the weekly update

Only the page creator can stop or add listening agents. On focus change also
saves on window blur, and Auto Save pauses on conflicts. Taken-down pages do
not unfurl. Model picker coverage matches agent-models.ts.

This branch was previously deployed

1 inactive deployment
preview — 29fa64da Deployed Sep 30, 2026 by saddlepaddle via Deploy Admin #10624
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