fix(file,camera): workspace write integrity

[adriannoes]

Summary

• Rewrite of workspace write integrity against current development (not a cherry-pick of the Cursor drafts).
• write_file maps to the intended path (membership ≠ target), rejects dangling workspace symlinks (lstat + O_NOFOLLOW), and persists via temp+fsync+rename.
• Camera capture fails closed when workspace_only is on with an empty workspace_path, and rejects dangling capture outputs.

Supersedes #67, #78, #90, #91.

Does not mix gateway, sandbox, or cron.

Test plan

• make test_file / ./build/test_file
• make test_hardware_camera / ./build/test_hardware_camera
• CI=true make test
• make static

[cursor]

Summary

Solid, scoped rewrite of workspace write integrity on development. write_file no longer treats ancestor membership as the fopen target (#67), rejects leaf dangling/symlink writes instead of creating host files (#90), and persists via temp+fsync+rename so a failed write cannot O_TRUNC the live file (#78). Camera now distinguishes set_workspace(NULL) from set_workspace("") (#91) and denies leaf dangling capture outputs. No merge blockers from this review.

Local checks (this image has no libcurl, so LDLIBS=-lm): CI=true make test_file / test_hardware_camera / test_hardware_tools all pass. test_file reports 47 tests, 0 failed. Same three binaries pass GCC ASan+UBSan.

Must Fix

None.

Should Fix

1. write_file_atomic always uses path.tmp. Writing notes.md O_TRUNCs an existing workspace notes.md.tmp, then renames it over notes.md. That is not the #78 live-target wipe, but it is silent loss of a sibling the agent may already have written. Use mkstemp in the parent directory so the sidecar cannot collide (src/tools/file.c around the snprintf(..., "%s.tmp", path) call).
2. path_is_dangling_symlink is lstat + S_ISLNK. Every symlink is denied, including an in-workspace alias to a regular file. Fail-closed is fine; the name and CHANGELOG ("dangling") are not. Rename the helper, or only reject when the target does not resolve, and add a test for ln -s notes.md alias.md then write_file alias.md (src/tools/file.c).

Nice to Have

• write_all treats write() -1 as fatal with no EINTR retry. A signal during the copy discards the temp and leaves the original (fail-closed, not data loss). Retrying EINTR would match how this tree already treats short writes.
• After rename, the parent directory is not fsynced. Crash durability of the directory entry is weaker than the file payload; #78 is still fixed because the live inode is never truncated in place.
• Camera still has no O_NOFOLLOW on the output path. Leaf dangling is denied before spawn, and agent_lock makes a same-process TOCTOU between check and gst/v4l2 hard. A second local uid racing a symlink into the path could still be followed by the CLI. Residual, not the #90 sequential attack.
• path_within_workspace still snprintfs into PATH_MAX without an overflow fail. resolve_workspace_write_path already rejects a truncated copy. Writes stay fail-closed; reads could still membership-walk a truncated string.
• test_hardware_tools covers outside-workspace capture, not the empty-workspace_path wiring in tool_hardware_set_config. The camera unit test covers set_workspace("").

Positive Highlights

• Membership vs target is actually split: resolve_workspace_write_path requires the immediate parent to realpath as a directory under the workspace, so important.md/nested.txt cannot truncate important.md, and missing_dir/notes.md cannot collapse onto workspace notes.md. Requiring the immediate parent also blocks the untested two-level case (a/b/c.txt onto a/c.txt) that the old "append basename to ancestor dir" path would have hit.
• The chmod 0555 regression is a real discriminator, not a coverage dummy. Creating path.tmp needs directory write mode; fopen(live, "w") on an existing 0644 file does not. If the old truncate-in-place code were still there, that test would pass the write and fail the preserve assertion.
• Camera NULL vs "" matches write_file when workspace_only is on and the root is missing, and tool_hardware_set_config passes "" instead of a NULL path. CHANGELOG Unreleased records all three bugs. Scope stays on file + camera.

  

_{Sent by Cursor Automation: Adrianno’s personal code review}