Skip to content

fix: confine editor workspace files to the selected worktree - #223

Merged
shiqui merged 1 commit into
mainfrom
codex/vuln-604-workspace-containment
Nov 4, 2026
Merged

shiqui merged 1 commit into
mainfrom
codex/vuln-604-workspace-containment

Conversation

@shiqui

@shiqui shiqui commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Description

Configured editor workspace files can currently resolve outside the selected worktree. For example, with a workspace-capable editor already selected, editor.workspace = ../../outside.code-workspace passes that existing external file to the editor.

Require workspace files from every config source to resolve inside the physical worktree root, including symlink targets. Invalid or missing explicit paths open the worktree folder without triggering auto-detection. Preserve nested paths, spaces, internal symlinks, configuration precedence, root auto-detection, and none. Valid paths retain their spelling so workspace-relative editor settings keep their meaning.

Motivation

Related: VULN-604.

A benign fake editor reproduced external-file argument selection on 2d4852d. The demonstrated impact is local argument selection; no GTR shell execution or actual editor trust bypass was demonstrated. The shared workspace setting remains data, while selecting an editor through shared defaults.editor still requires command approval.

Type of Change

  • Breaking change: previously accepted outside-worktree workspace paths now open the folder.
  • Documentation update.
  • Regression coverage for workspace path containment.

Testing

After rebasing onto 801f33c:

  • bats tests/workspace.bats tests/adapters.bats tests/launch.bats tests/cmd_help.bats: 64 passed, including 18 new workspace regressions.
  • Repository-wide ShellCheck: passed.
  • ./scripts/generate-completions.sh --check: passed.
  • Bash 3.2 syntax checks and git diff --check: passed.

Earlier validation on the original 2d4852d base:

  • Full bats tests/ run: 609 passed, 12 failed. All 12 failures also reproduced on an unmodified baseline in this macOS environment: three PR/SSH URL cases, two debug-trap cases, six generated-shell/fish cases, and one macOS terminal-quoting case. This is not a green full-suite claim for the rebased branch.
  • Benign fake-editor smoke through git gtr new --editor, plus config, go, list, run, and removal: passed on macOS Bash 3.2.

Manual Testing Checklist

  • macOS with system Bash 3.2.
  • Create, navigate, open in a fake editor, list, run, configure, and remove worktrees.
  • Native Linux.
  • Native Windows (separator handling has simulated regression coverage only).

Breaking Changes

Outside-worktree paths in local/global Git config as well as shared config now fall back to opening the folder. Configure a workspace file located inside the selected worktree, or open an external workspace directly in the editor. README, configuration documentation, and CLI help describe the policy.

Checklist

  • Read CONTRIBUTING.md and followed repository style and compatibility requirements.
  • Added regression coverage and performed isolated manual testing.
  • Updated documentation.
  • No new dependencies.
  • Completion output verified; no commands or flags added.
  • Platform limitations and existing test failures noted.

Additional Context

Containment checks the workspace file itself, not the folders or settings described inside it. Editor trust remains the editor's responsibility. Validation is not atomic with editor launch and cannot prevent filesystem changes after the check. No real-editor trust UI, live exploits, or customer data were used.

License Acknowledgment

This contribution is submitted under the Apache License 2.0.

Summary by CodeRabbit

  • New Features

    • Workspace settings now accept files only when they resolve within the selected worktree, including through symlinks. Invalid or missing configured paths open the worktree folder instead of triggering auto-detection.
    • Root-level workspace files can still be detected automatically when no explicit path is configured.
    • Shared workspace settings do not require command approval; editor selection through .gtrconfig still requires trust.
  • Documentation

    • Clarified workspace path rules, fallback behavior, and editor trust requirements.

@linear

linear Bot commented Nov 4, 2026

Copy link
Copy Markdown

VULN-604

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Workspace file resolution now rejects paths that do not resolve inside the selected worktree, including symlink targets. Invalid explicit paths open the worktree folder without auto-detection. Documentation and regression tests cover the policy and supported path cases.

Changes

Editor workspace selection

Layer / File(s) Summary
Validate and resolve workspace paths
lib/adapters.sh, README.md, docs/configuration.md, lib/commands/help.sh
Configured and auto-detected workspace files now pass containment checks against the physical worktree root. Invalid explicit paths produce no workspace path and prevent auto-detection. Documentation and help describe the behavior and command-approval distinction.
Cover workspace selection cases
tests/workspace.bats
New tests cover containment, symlinks, path edge cases, configuration precedence, auto-detection, and shared workspace settings.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: natoboram

Merge Risk: Minimal · up to 6755c

No unresolved workspace-selection defect was established; the change is mergeable after normal checks.

Security architecture risk: Minimal · up to 6755c

The change restricts workspace-file selection while preserving separate editor-command approval and folder fallback. No material security risk introduced or worsened by this PR was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A contributor controlling shared workspace configuration can influence which eligible file is passed to an already-selected local editor. The new gate narrows ordinary path selection to the selected physical worktree; it does not grant command approval or establish that workspace contents are safe.

Security Findings and Attack Paths

  • inferred — Containment is a path check, not an atomic handoff of the checked file. An attacker with concurrent write access could replace a symlink before the editor resolves the returned original path. This residual condition is not a new PR concern: the base already allowed external targets and used the same path-based launch interface.

Trust Boundaries and Controls

  • observed — Shared editor.workspace remains unapproved configuration data, while shared defaults.editor still requires trusted command definitions. Adapter loading separately parses and validates the selected executable command, preserving the distinction between file selection and executable authority.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "fix: confine editor workspace files to the selected worktree" matches the main change. The raw summary and objectives both describe requiring configured and auto-detected workspace files to…
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.
Full details: Title check

Explanation

The title "fix: confine editor workspace files to the selected worktree" matches the main change. The raw summary and objectives both describe requiring configured and auto-detected workspace files to resolve inside the selected worktree, including symlink targets, with invalid paths falling back to opening the worktree folder. The title is specific, short, and uses a conventional commit prefix, so a teammate scanning history would understand the primary change.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 3 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks each workspace path,
And keeps each file within its home.
Through nested folders, links may pass,
While outside paths leave none to roam.
The worktree opens, safe and calm.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 801f33c and 6755c56.

Files selected for processing: 5
  • README.md
  • docs/configuration.md
  • lib/adapters.sh
  • lib/commands/help.sh
  • tests/workspace.bats

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

@shiqui
shiqui merged commit 7c3d990 into main Nov 4, 2026
5 checks passed
@shiqui
shiqui deleted the codex/vuln-604-workspace-containment branch November 4, 2026 18:09
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