Skip to content

fix(agents): tighten workspace file opens#66079

Open
eleqtrizit wants to merge 8 commits intomainfrom
423
Open

fix(agents): tighten workspace file opens#66079
eleqtrizit wants to merge 8 commits intomainfrom
423

Conversation

@eleqtrizit
Copy link
Copy Markdown
Contributor

Summary

  • Routes agents.files.get, agents.files.set, and workspace file listing through the existing root-scoped fs-safe helpers
  • Removes the separate path-resolution layer for allowlisted workspace files so the gateway uses the same open/read/write validation path everywhere

Changes

  • Replaced the hand-rolled workspace file resolver in src/gateway/server-methods/agents.ts with openFileWithinRoot, readFileWithinRoot, and writeFileWithinRoot
  • Updated src/infra/fs-safe.ts to resolve opened file real paths from the open file descriptor before falling back to the current path
  • Tightened the gateway tests to reject symlink aliases for allowlisted agent files and added an fs-safe regression that locks in the fd-first realpath behavior

Validation

  • Ran pnpm test src/gateway/server-methods/agents-mutate.test.ts
  • Ran pnpm test src/infra/fs-safe.test.ts
  • Ran claude -p "/review" and addressed the actionable test-feedback item by switching the mocks to real SafeOpenError instances
  • Ran pnpm check on the rebased branch; it still fails in unrelated existing files outside this change set: extensions/discord, extensions/feishu, extensions/nextcloud-talk, extensions/whatsapp, src/cron, src/mcp, and src/wizard

Notes

  • This change intentionally rejects allowlisted agent-file symlink aliases instead of resolving through them
  • I did not broaden scope into the unrelated repo-wide pnpm check failures above

@openclaw-barnacle openclaw-barnacle bot added gateway Gateway runtime size: L maintainer Maintainer-authored PR labels Apr 13, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 13, 2026

Greptile Summary

This PR tightens workspace file access in the agents gateway by routing all agents.files.get, agents.files.set, and agents.files.list paths through the root-scoped fs-safe helpers (openFileWithinRoot, readFileWithinRoot, writeFileWithinRoot), removing the old hand-rolled resolveAgentWorkspaceFilePath resolver. It also switches resolveOpenedFileRealPathForHandle to fd-first path resolution (trying /proc/self/fd/${fd} before fs.realpath) to harden against symlink-swap races, adds nonBlockingRead: true throughout to prevent FIFO blocking, properly gates test hooks to test environments, and closes the handle on afterOpen hook failures. All previous review findings have been addressed.

Confidence Score: 5/5

Safe to merge — all prior review findings addressed, no new P0/P1 issues found, security hardening is correct and well-tested.

All previous P1 and P2 findings from prior review rounds have been resolved. The fd-first realpath resolution, nonBlockingRead coverage, identity-check restoration for allowSymlinkTargetWithinRoot, handle cleanup on afterOpen failures, and test-hook environment guard are all correctly implemented. The remaining changes are straightforward consolidations with good test coverage. Only P2 or lower findings remain.

No files require special attention.

Reviews (5): Last reviewed commit: "fix(worklog): append comment resolution ..." | Re-trigger Greptile

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 87390e0990

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a9253ffb4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a9253ffb4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a9253ffb4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2395311e4d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 81b70a145e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

for (const fdPath of fdCandidates) {
try {
return await fs.realpath(fdPath);

P1 Badge Restore final pathname identity check after fd resolution

By resolving /proc/self/fd/... before ioPath here, the post-open validation no longer confirms that the original pathname still refers to the opened inode at the end of verification. A swap in the narrow window after the lstat/stat identity check (for example, renaming the original file and replacing the original path with a symlink) will still pass because validation follows the fd target, not the current pathname. This reintroduces stale-read/path-mismatch risk for path-sensitive callers such as script preflight (validateScriptFileForShellBleed) that read by path and then execute that path. Fresh evidence: this commit moved the ioPath realpath attempt below the fd loop, which removed the previous pathname-based detection step.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@eleqtrizit
Copy link
Copy Markdown
Contributor Author

@greptile review

@eleqtrizit
Copy link
Copy Markdown
Contributor Author

@codex review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime maintainer Maintainer-authored PR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant