Skip to content

Refuse secret key/token writes through a planted dest symlink - #365

Merged
tylerreckart merged 3 commits into
mainfrom
fix/secret-write-dest-nofollow
Sep 21, 2026
Merged

tylerreckart merged 3 commits into
mainfrom
fix/secret-write-dest-nofollow

Conversation

@cursor

@cursor cursor Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Problem

write_key_file (setup wizard / set_search_api_key) and admin-token generate (arbiter --api) opened the dest with open(O_WRONLY|O_CREAT|O_TRUNC). That follows a symlink. A planted ~/.arbiter/openrouter_api_key, search_api_key, or admin_token → elsewhere link was truncated with the secret. Reproduced: admin_token → secret.txt left the victim as adm_LEAKED_TOKEN.

Path chmod(0600) after close had the same follow-the-link hole. A planted FIFO could hang the wizard / --api startup or deliver the secret to a waiting reader.

Fix

Shared write_secret_file opens with O_WRONLY|O_CREAT|O_TRUNC|O_NOFOLLOW|O_CLOEXEC|O_NONBLOCK, fstats a regular file, fchmod(0600) (no path chmod), writes with EINTR retry, and fsyncs. Isolated new helper used by write_key_file and resolve_admin_token.

Tests

unit_secret_file — create 0600 file, overwrite + tighten 0644, planted dest symlink does not clobber the target (victim stays do-not-clobber), planted FIFO refused without hanging, directory dest refused. 5/5 locally (18 assertions) ×3 + ASan + UBSan + TSan. arbiter compiles.

Independently mergeable against main (0456350). git merge-tree --write-tree vs main, #321, #323, #326, #339, #342, #343, #344, #351, #354–#364 CLEAN. Does not overlap #363 (atomic_file tmp staging) or #364 (MCP registry tmp). CHANGELOG [Unreleased] is the usual sibling note.

Open in Web View Automation 

Note

High Risk
Changes how API keys and admin tokens are persisted; incorrect open flags or validation could break setup/--api startup or leave secrets writable via symlink tricks.

Overview
Fixes a local symlink/FIFO exfiltration hole when arbiter writes secrets under ~/.arbiter/. Setup wizard API-key saves (write_key_file) and fresh admin token generation on arbiter --api no longer use open(O_CREAT|O_TRUNC) (which followed a planted dest symlink) or post-close chmod on the path.

A shared write_secret_file helper opens with O_NOFOLLOW|O_NONBLOCK, requires a regular file via fstat, sets mode 0600 with fchmod, writes with retry/fsync, and fails closed on symlink, FIFO, or directory destinations. unit_secret_file covers create/overwrite, symlink non-clobber, FIFO no-hang, and directory refusal. CHANGELOG [Unreleased] documents the behavior.

Reviewed by Cursor Bugbot for commit bf38a8c. Bugbot is set up for automated code reviews on this repo. Configure here.

cursoragent and others added 2 commits September 21, 2026 00:16
write_key_file and admin-token generate opened the dest with
O_CREAT|O_TRUNC, which follows a symlink. A planted
~/.arbiter/openrouter_api_key, search_api_key, or admin_token link
redirected the secret into the target. Shared write_secret_file opens
with O_NOFOLLOW and requires a regular file so a dest symlink or FIFO
cannot exfiltrate the bytes.

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
@tylerreckart
tylerreckart marked this pull request as ready for review September 21, 2026 12:58
@tylerreckart
tylerreckart enabled auto-merge (squash) September 21, 2026 12:58
Keep both Unreleased notes after #366 landed on main (CHANGELOG only).

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
@tylerreckart
tylerreckart merged commit 5fc72f6 into main Sep 21, 2026
cursor Bot pushed a commit that referenced this pull request Sep 21, 2026
Keep the LaTeX depth-cap Unreleased bullet alongside the secret-write
and --connect URL fixes that landed on main (#365, #366).

Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
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.

3 participants