Enforce AllowedPaths read/write modes by matt-dz · Pull Request #271 · DataDog/rshell

matt-dz · 2026-05-29T18:27:12Z

Summary

This updates AllowedPaths to support and enforce optional terminal path mode suffixes:

no suffix and :ro configure a read-only root
:rw configures a root that may be written when the sandbox is placed in writable/remediation mode
overlapping roots use the most-specific configured path, so a narrower :ro overrides a broader :rw
POSIX literal paths ending in :ro or :rw are preserved when the literal path exists; Windows strips terminal mode suffixes directly

The write path now enforces those modes for both Open and Truncate. Write targets are checked after symlink/canonical resolution, Unix write opens use a no-follow openat walk, Windows uses Go no-reparse os.Root.OpenFile, and canonical symlink roots are verified against the already-opened root handle before they are used for write authorization.

Only the supported platform implementations are kept: Linux/macOS use the Unix openat implementation, and Windows uses the Windows os.Root implementation. There is no generic unsupported-platform write opener.

Validation

make fmt
go test ./allowedpaths ./analysis
go test ./interp ./builtins/tests/truncate ./tests
go test ./...
RSHELL_BASH_TEST=1 go test ./tests/ -run TestShellScenariosAgainstBash -timeout 120s
env GOTOOLCHAIN=local GOCACHE=/private/tmp/rshell-go-cache GOOS=windows GOARCH=amd64 go test -c ./allowedpaths -o /private/tmp/rshell-allowedpaths-windows.test.exe
git diff --check

Notes

The commit was created unsigned in the Codex execution environment because dd-gitsign could not access an SSH agent (SSH_AUTH_SOCK was unset).

datadog-prod-us1-3 · 2026-05-29T18:27:47Z

✨ Fix all issues with BitsAI

⚠️ Warnings

🚦 1 Pipeline job failed

Static Analysis Verification | Static Analysis Label Check

Useful? React with 👍 / 👎

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 0f8bec6 | Docs | Datadog PR Page | Give us feedback!}

matt-dz · 2026-05-29T18:40:33Z

@codex review

Please review this PR for correctness, security regressions around AllowedPaths path-mode parsing, most-specific root matching, write-surface preservation, cross-platform behavior, and missing test coverage.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0fd9d8702

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

matt-dz · 2026-05-29T18:49:47Z

@codex review

Pushed 6fcaad31 to address the literal :ro/:rw path compatibility finding. Existing suffixed literal paths are now preserved as read-only; suffix parsing only applies when the literal path is absent.

Validation:

make fmt
go test ./allowedpaths
go test ./analysis
go test ./interp
go test ./...

matt-dz · 2026-05-29T18:56:20Z

@codex review

Pushed 61e87e13 after the compatibility fix to skip the POSIX-only colon-literal regression tests on Windows. The production behavior is unchanged from 6fcaad31; this only avoids invalid Windows test directory names.

Validation:

make fmt
go test ./allowedpaths
go test ./analysis
go test ./interp
env GOTOOLCHAIN=local GOCACHE=/private/tmp/rshell-go-cache GOOS=windows GOARCH=amd64 go test -c ./allowedpaths -o /private/tmp/rshell-allowedpaths-windows.test.exe
go test ./...

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 61e87e1375

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

matt-dz · 2026-05-29T19:05:30Z

@codex review

Pushed 6cf4c4dc to address the Windows path-mode suffix finding. Path-mode resolution is now split by platform: POSIX/non-Windows preserves existing literal suffixed paths, while Windows strips terminal :ro/:rw suffixes directly and has a Windows-only regression test for that behavior.

Validation:

make fmt
go test ./allowedpaths
go test ./analysis
go test ./interp
env GOTOOLCHAIN=local GOCACHE=/private/tmp/rshell-go-cache GOOS=windows GOARCH=amd64 go test -c ./allowedpaths -o /private/tmp/rshell-allowedpaths-windows.test.exe
go test ./...

chatgpt-codex-connector · 2026-05-29T19:08:27Z

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

matt-dz · 2026-06-22T15:26:56Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6cfa761ad9

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

matt-dz · 2026-06-22T15:44:44Z

@codex review

Pushed 2abf4338 to enforce :rw path modes at sandbox write boundaries and added regression coverage for read-only roots in remediation mode.

Validation:

make fmt
go test ./allowedpaths
go test ./analysis ./interp
go test ./builtins/tests/truncate ./tests
go test ./...

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2abf43382b

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

matt-dz · 2026-06-22T15:54:30Z

@codex review

Pushed 3f733b9d to enforce path modes after resolving in-root symlink write targets, closing the symlink bypass into narrower read-only roots.

Validation:

make fmt
go test ./allowedpaths
go test ./analysis ./interp
go test ./builtins/tests/truncate ./tests
go test ./...
env GOTOOLCHAIN=local GOCACHE=/private/tmp/rshell-go-cache GOOS=windows GOARCH=amd64 go test -c ./allowedpaths -o /private/tmp/rshell-allowedpaths-windows.test.exe
git diff --check

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f733b9d6d

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

matt-dz · 2026-06-22T16:34:25Z

@codex review

Pushed 17902805 to enforce write path modes against canonical root spellings too, including symlinked allowed roots that point at real paths configured read-only.

Validation:

make fmt
go test ./allowedpaths
go test ./...
env GOTOOLCHAIN=local GOCACHE=/private/tmp/rshell-go-cache GOOS=windows GOARCH=amd64 go test -c ./allowedpaths -o /private/tmp/rshell-allowedpaths-windows.test.exe
git diff --check

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17902805f8

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

matt-dz · 2026-06-22T16:56:06Z

@codex review

Pushed d0ca37c5 to make Unix write opens use a no-follow openat walk after the path-mode decision, closing the symlink swap race into read-only child roots.

Validation:

make fmt
go test ./allowedpaths ./analysis
go test ./interp
go test ./builtins/tests/truncate ./tests
go test ./...
env GOTOOLCHAIN=local GOCACHE=/private/tmp/rshell-go-cache GOOS=windows GOARCH=amd64 go test -c ./allowedpaths -o /private/tmp/rshell-allowedpaths-windows.test.exe
git diff --check

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0ca37c562

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

matt-dz · 2026-06-22T17:18:20Z

@codex review

Pushed 95293912 to address the latest review findings: read-only/default roots no longer allocate Unix write-root fds, and scenario AllowedPaths now preserve YAML path-mode semantics without a remediation-mode :rw harness rewrite.

Validation:

make fmt
go test ./allowedpaths ./tests
go test ./analysis ./interp
go test ./builtins/tests/truncate
go test ./...
RSHELL_BASH_TEST=1 go test ./tests/ -run TestShellScenariosAgainstBash -timeout 120s
env GOTOOLCHAIN=local GOCACHE=/private/tmp/rshell-go-cache GOOS=windows GOARCH=amd64 go test -c ./allowedpaths -o /private/tmp/rshell-allowedpaths-windows.test.exe
git diff --check

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 95293912e4

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

matt-dz · 2026-06-22T17:33:58Z

@codex review

Pushed bd3174ba to bind Unix write roots to the already-opened sandbox root instead of reopening the configured pathname, closing the construction-time root-swap race.

Validation:

make fmt
go test ./allowedpaths ./analysis
go test ./interp ./builtins/tests/truncate ./tests
go test ./...
RSHELL_BASH_TEST=1 go test ./tests/ -run TestShellScenariosAgainstBash -timeout 120s
env GOTOOLCHAIN=local GOCACHE=/private/tmp/rshell-go-cache GOOS=windows GOARCH=amd64 go test -c ./allowedpaths -o /private/tmp/rshell-allowedpaths-windows.test.exe
git diff --check

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bd3174badf

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

matt-dz · 2026-06-22T17:56:53Z

@codex review

chatgpt-codex-connector · 2026-06-22T18:10:59Z

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 6033b6a0ee

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

julesmcrt

Few nits, overall LGTM

julesmcrt · 2026-06-24T09:40:56Z

 // AllowedPaths restricts file and directory access to the specified directories.
-// Paths must be absolute directories that exist. When set, only files within
-// these directories can be opened (for reading or writing), read, or executed.
+// Paths must be absolute directories that exist. A path may end with :ro or :rw


CLI --allowed-paths help text not updated for :ro/:rw suffixes

go run ./cmd/rshell --help outputs:

... -p, --allowed-paths string comma-separated list of directories the shell is allowed to access ...

❓ Should we make the suffix appear here?

See cmd/rshell/main.go:142,147

julesmcrt · 2026-06-24T09:45:48Z

+
+package allowedpaths
+
+func resolveAllowedPathMode(path string) (string, pathMode) {


Not a blocker as we don't plan on supporting Windows for now.

❓ Should we add a comment documenting that ADS-style suffixes are intentionally treated as mode suffixes on Windows, so the divergence from the Unix literal-preservation behavior is explicit.

A path like C:\data\file:rw is ambiguous with an NTFS Alternate Data Stream (file:rw), so the Windows code strips unconditionally.

However the Unix resolveAllowedPathMode carefully preserves a literal path that genuinely ends in :ro/:rw (via os.Lstat), so a real directory named tenant:rw is never silently reinterpreted as a mode.

julesmcrt · 2026-06-24T09:46:21Z

 	"os.Getuid",                          // 🟠 returns the numeric user id of the caller; read-only syscall.
 	"os.O_APPEND",                        // 🟢 append-on-write file flag constant; pure constant. Part of the sandbox open-flag allowlist.
 	"os.O_CREATE",                        // 🟢 create-if-missing file flag constant; pure constant. Part of the sandbox open-flag allowlist.
+	"os.Lstat",                           // 🟠 checks whether a literal AllowedPaths entry exists without following symlinks, preserving legacy paths that end in :ro/:rw.


Shouldn't we preserve alphabetical order here?

chatgpt-codex-connector Bot reviewed May 29, 2026

View reviewed changes

Comment thread allowedpaths/path_mode.go Outdated

chatgpt-codex-connector Bot reviewed May 29, 2026

View reviewed changes

Comment thread allowedpaths/sandbox.go Outdated

matt-dz changed the title ~~[codex] Parse path access modes in allowed paths~~ Parse path access modes in allowed paths Jun 22, 2026

matt-dz added 4 commits June 22, 2026 10:58

Parse path access modes in allowed paths

107b526

Preserve literal allowed paths with mode suffixes

dc6b7f8

Skip POSIX literal path tests on Windows

6e69398

Handle Windows path mode suffixes

819c858

matt-dz force-pushed the codex/rshell-path-modes-poc branch from 6cf4c4d to 819c858 Compare June 22, 2026 15:05

matt-dz added 3 commits June 22, 2026 11:15

add comment on non-windows filecheck for suffix split

e082e22

test: add tests

e3c7f8e

style: remove merge marker

6cfa761

chatgpt-codex-connector Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread allowedpaths/sandbox.go Outdated

Enforce read-write allowed path modes

2abf433

chatgpt-codex-connector Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread allowedpaths/sandbox.go Outdated

Enforce path modes through symlinks

3f733b9

chatgpt-codex-connector Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread allowedpaths/sandbox.go

Respect canonical path modes for writes

1790280

chatgpt-codex-connector Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread allowedpaths/sandbox.go

Avoid symlink races for write opens

d0ca37c

chatgpt-codex-connector Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread allowedpaths/sandbox.go Outdated

Comment thread tests/scenarios_test.go Outdated

Avoid write handles for read-only roots

9529391

chatgpt-codex-connector Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread allowedpaths/sandbox.go Outdated

Bind write roots to opened sandbox roots

bd3174b

chatgpt-codex-connector Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread allowedpaths/sandbox.go

Comment thread allowedpaths/internal/writeopen/writeopen_fallback.go

Verify canonical roots against opened handles

6033b6a

matt-dz changed the title ~~Parse path access modes in allowed paths~~ Enforce AllowedPaths read/write modes Jun 22, 2026

Remove unsupported writeopen fallback

0f8bec6

matt-dz marked this pull request as ready for review June 22, 2026 20:13

matt-dz requested review from AlexandreYang, astuyve, julesmcrt, thieman and val06 as code owners June 22, 2026 20:13

matt-dz enabled auto-merge June 23, 2026 17:22

julesmcrt approved these changes Jun 24, 2026

View reviewed changes

matt-dz added this pull request to the merge queue Jun 24, 2026

Merged via the queue into main with commit b6ca032 Jun 24, 2026
40 of 41 checks passed

matt-dz deleted the codex/rshell-path-modes-poc branch June 24, 2026 09:47

matt-dz mentioned this pull request Jun 24, 2026

Address AllowedPaths follow-up nits #541

Merged


		package allowedpaths

		func resolveAllowedPathMode(path string) (string, pathMode) {

Uh oh!

Conversation

matt-dz commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Notes

Uh oh!

datadog-prod-us1-3 Bot commented May 29, 2026 • edited by datadog-prod-us1-5 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Warnings

Uh oh!

matt-dz commented May 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

matt-dz commented May 29, 2026

Uh oh!

matt-dz commented May 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

matt-dz commented May 29, 2026

Uh oh!

chatgpt-codex-connector Bot commented May 29, 2026

Uh oh!

matt-dz commented Jun 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

matt-dz commented Jun 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

matt-dz commented Jun 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

matt-dz commented Jun 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

matt-dz commented Jun 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

matt-dz commented Jun 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

matt-dz commented Jun 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

matt-dz commented May 29, 2026 •

edited

Loading

datadog-prod-us1-3 Bot commented May 29, 2026 •

edited by datadog-prod-us1-5 Bot

Loading