fix(agent): descriptor-rooted, no-follow file operations

[ValgulNecron]

Summary

The agent file browser (agent/internal/files) used to validate a path and then reopen it by string. A component swapped for a symlink between the check and the open could therefore redirect a read, write or delete outside the data root (TOCTOU). Every operation now runs through directory descriptors opened from a trusted root descriptor and refuses links at every component.

• Walk: resolve() is now purely lexical; it does the dotfile and ../escape checks. Each directory component is opened with openat(O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC) relative to its parent's descriptor. The final component is opened with O_NOFOLLOW.
• Links: any symlink is refused with 400, whether it points inside or outside the root. The target is only read to pick the message, so "dotfile access denied" stays byte-exact.
• Mutations:
  • Writes and uploads: a temp file is created through the parent descriptor (O_EXCL|O_NOFOLLOW), then fchmod, fsync and renameat in the same directory.
  • mkdir and delete: mkdirat and unlinkat. Recursive delete walks descriptors.
• Reads, downloads and listing: reads and downloads are served from the opened descriptor via ServeContent; listing reads the opened directory descriptor.
• Preserved: dotfile rules, size limits (2 MiB read), atomic writes, root-delete refusal, and status codes and bodies. Read and download open failures still go through httpErr ("not found" / "forbidden" / "internal error").
• Why not os.Root or openat2: os.Root follows in-root symlinks. openat2(RESOLVE_NO_SYMLINKS) needs kernel 5.6+ plus this same walk as a fallback.
• Lint: fd → *os.File goes through one fileFromFD helper with an fd < 0 guard, so gosec G115 is satisfied.

Behaviour changes on purpose:

• In-root symlinks are no longer followed for read, list, download, write, upload, mkdir, or delete through a link. The body is "path escapes root (symbolic link in path)".
• Writes are fsynced.
• The files package builds only on Linux (rooted_linux.go). The agent ships only as Linux images.

Tests

• rooted_test.go (new): race tests use a synchronous beforeOpen seam. It swaps a checked directory for a symlink to outside the root just before the final open. The tests check that read, list, write, mkdir, upload and delete all fail and never touch anything outside the root. There are also unit tests for the walk helpers and for fileFromFD.
• Existing tests changed, approved by the maintainer (files_test.go, nothing deleted):
  • TestResolve_SymlinkEscape now asserts through h.list.
  • TestResolve_SymlinkToDotfileRejected asserts through read, write and delete, with stronger checks.
  • Three tests asserted that in-root symlinks are followed. They are inverted to …RejectsInRootSymlinkDestination / TestUpload_RejectsSymlinkDestinationInRoot. The "symlinked root still serves" half moved to TestRooted_SymlinkedRootStillServes.

Test plan

• go (agent / amd64|arm64) and lint (gosec, noctx, revive)
• Coverage gate agent 90
• e2e api-agent bucket (file browser over the agent)

Nothing was built or run locally, at the maintainer's request; CI is the verification. Briefed at sonnet, implemented at haiku, reviewed at opus; the review fixes were verified at sonnet.

🤖 Generated with Claude Code

https://claude.ai/code/session_018MnGTGHzHhFQCa22wRknUS

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: ValgulNecron/Gameplane/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: c326ea7d-84d4-4de6-ab83-a93d6ffc2434

📥 Commits

Reviewing files that changed from the base of the PR and between 14521d7 and 407ba35.

📒 Files selected for processing (6)

• agent/internal/files/files.go
• agent/internal/files/files_test.go
• agent/internal/files/rooted_linux.go
• agent/internal/files/rooted_test.go
• agent/specs.md
• docs/security.md

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

CI Report - 407ba35

run 37342845735 - attempt 1

0 failed - 14 passed - 6 skipped - 27m54s elapsed

Coverage

no coverage/* commit statuses found on 407ba35

Changed

Area	Files	+/-
agent/	5	+1268/-341
docs/	1	+29/-0

Slowest jobs

Job	Duration
e2e game bot (kind)	20m14s
e2e multicluster / amd64 (kind x2)	10m38s
e2e multicluster / arm64 (kind x2)	9m42s
go (operator / amd64)	8m7s
e2e operator / arm64 (kind)	7m36s

e2e buckets run

api-agent, api-auth, api-mods, api-rbac, api-roles, bot-fast, multicluster, operator, ratelimit, upgrade