fix(eval): trial log dirs are world-writable

[SebTardif]

What does this PR do?

Trial directories created for native eval are no longer world-writable (0o777). They are created as 0o755 so only the owner can write logs and artifacts.

Why?

DockerTaskEnvironment.start chmod'd agent, verifier, and artifacts/logs/artifacts to 0o777. On a shared host another user could plant or rewrite trial logs. Cleanup paths in the same tree already use 0o755 for directories.

Changes

• Add prepare_trial_dirs() and chmod 0o755 instead of 0o777
• Cover the mode in test_prepare_trial_dirs_are_owner_writable_only

Tests

• Focused check tests/test_native_eval_runner.py::test_prepare_trial_dirs_are_owner_writable_only passes locally
• python -m ruff check clawbench app.py scripts tests passes locally

Red: with 0o777 the new check reports mode 511 (0o777) instead of 493 (0o755). Green: dirs are 0o755.

$ python3 -m pytest -q tests/test_native_eval_runner.py::test_prepare_trial_dirs_are_owner_writable_only
1 passed
$ python3 -m ruff check scripts/native_eval/runtime.py tests/test_native_eval_runner.py
All checks passed!

What Problem This Solves

Fixes an issue where native-eval trial log directories were world-writable, so another user on the same host could plant or rewrite agent, verifier, or artifact logs.

Evidence

Terminal output from the patched helper:

$ python3 -m pytest -q tests/test_native_eval_runner.py::test_prepare_trial_dirs_are_owner_writable_only
1 passed
mode 0o755

Real behavior proof

• Behavior or issue addressed: Native-eval start created trial dirs with mode 0o777, which lets any local user write logs.

• Real environment tested: macOS 15, Python 3.14, worktree /tmp/sb-chmod on fix/trial-dir-mode.

• Exact steps or command run after this patch:

  python3 -m pytest -q tests/test_native_eval_runner.py::test_prepare_trial_dirs_are_owner_writable_only

• Evidence after fix: terminal output from the patched helper. Trial dirs are owner-writable only:

  1 passed
  mode 0o755

• Observed result after fix: prepare_trial_dirs creates agent, verifier, and artifacts/logs/artifacts as 0o755.

• What was not tested: A live docker compose trial against a remote registry.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 5, 2026, 11:59 AM ET / 15:59 UTC.

ClawSweeper review

What this changes

The PR changes three native-evaluation log directories from world-writable to owner-writable permissions and adds a helper-level regression test.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Blocked before merge - 4 items remain

This hardening remains necessary on current main, but the unchanged branch has not resolved the prior compatibility finding or provided production-path proof.

Priority: P2
Reviewed head: f88349413ce4dbd046603c3b74851328af0fec02

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The patch is focused, but the prior compatibility blocker remains and its proof covers only a helper-level test.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: The macOS pytest transcript exercises prepare_trial_dirs and verifies modes, but never exercises DockerTaskEnvironment.start, its bind mounts, or successful container log and reward writes after the change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The macOS pytest transcript exercises prepare_trial_dirs and verifies modes, but never exercises DockerTaskEnvironment.start, its bind mounts, or successful container log and reward writes after the change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	8 items	Verified introduced change: The pinned merge-base-to-head diff changes chmod from 0777 to 0755, extracts directory preparation, and adds one mode-checking test; it introduces no ownership adjustment. Current main still needs hardening: Current main still applies chmod(0o777). Agent and verifier execution retain the configured container user unless an explicit override is supplied; their run_trial calls supply no override. Host permissions cross the Docker boundary: Both single-container and Compose startup bind-mount the three host directories under /logs. Neither path changes their ownership or replaces the task image/service user, so a different non-root UID loses directory write access under 0755.
Findings	1 actionable finding	[P1] [P1] Preserve writes for non-root task containers
Security	None	None.

How this fits together

ShellBench’s native evaluation runner starts Docker task environments and runs agents and verifiers inside them. Host directories mounted into those containers collect logs, artifacts, and rewards used to evaluate the run.

flowchart TD
  A[Task definition and container user] --> D[Docker task container]
  B[Host trial directory] --> C[Directory permissions]
  C --> D
  D --> E[Agent and verifier writes]
  E --> F[Logs artifacts and rewards]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The macOS pytest transcript exercises prepare_trial_dirs and verifies modes, but never exercises DockerTaskEnvironment.start, its bind mounts, or successful container log and reward writes after the change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• [P1] Preserve writes for non-root task containers (P1) - On Linux, a task using a non-root UID different from the host runner receives these bind mounts owned by the host UID. Neither Docker startup nor agent/verifier execution adjusts ownership or overrides the configured task user. Changing the directories to 0755 therefore makes /logs/agent/* redirections and verifier reward creation fail with permission denied, breaking previously writable task configurations. Preserve access for the configured container writer while excluding unrelated host users; the helper-only mode test does not cover this.
• Resolve merge risk (P1) - Existing Linux task configurations with differing non-root container UIDs can stop producing usable evaluations after upgrade; the supplied macOS helper test does not establish compatibility.
• Complete next step (P2) - Repair container write compatibility and add the real Docker evidence described above. Updating the PR body should trigger re-review automatically; otherwise ask a maintainer to comment @clawsweeper re-review.

Findings

• [P1] [P1] Preserve writes for non-root task containers — scripts/native_eval/runtime.py:113

Agent review details

Security

None.

Review metrics

None.

Merge-risk options

Maintainer options:

1. Preserve configured container writers (recommended)
   Repair access for differing container identities without restoring unrestricted host writes, then demonstrate existing task configurations still produce logs and rewards.

Technical review

Best possible solution:

Restrict unrelated host users while preserving writes by the host runner and configured container identity, with fresh-run and existing-task compatibility demonstrated.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: on Linux, host-owned 0755 bind mounts deny writes by a different non-root container UID, while agent and verifier commands require those writes. No runtime reproduction was executed in this read-only review.

Is this the best way to solve the issue?

No. The permission change addresses world-writable directories but omits the container writer’s identity; the existing archive-cleanup permissions do not establish compatibility for active log mounts.

Full review comments:

• [P1] [P1] Preserve writes for non-root task containers — scripts/native_eval/runtime.py:113
  On Linux, a task using a non-root UID different from the host runner receives these bind mounts owned by the host UID. Neither Docker startup nor agent/verifier execution adjusts ownership or overrides the configured task user. Changing the directories to 0755 therefore makes /logs/agent/* redirections and verifier reward creation fail with permission denied, breaking previously writable task configurations. Preserve access for the configured container writer while excluding unrelated host users; the helper-only mode test does not cover this.
  Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against c1a79f731541.

Labels

Label justifications:

• P2: This is useful shared-host permission hardening with a bounded native-evaluation scope and no demonstrated urgent production incident.
• merge-risk: 🚨 compatibility: Changing bind-mounted directories to 0755 without accommodating container ownership can break existing non-root task configurations.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The macOS pytest transcript exercises prepare_trial_dirs and verifies modes, but never exercises DockerTaskEnvironment.start, its bind mounts, or successful container log and reward writes after the change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Verified introduced change: The pinned merge-base-to-head diff changes chmod from 0777 to 0755, extracts directory preparation, and adds one mode-checking test; it introduces no ownership adjustment. (scripts/native_eval/runtime.py:113, f88349413ce4)
• Current main still needs hardening: Current main still applies chmod(0o777). Agent and verifier execution retain the configured container user unless an explicit override is supplied; their run_trial calls supply no override. (scripts/native_eval/runtime.py:119, c1a79f731541)
• Host permissions cross the Docker boundary: Both single-container and Compose startup bind-mount the three host directories under /logs. Neither path changes their ownership or replaces the task image/service user, so a different non-root UID loses directory write access under 0755. (scripts/native_eval/runtime.py:164, f88349413ce4)
• Writes are required during execution: Harness commands redirect output into /logs/agent before the agent starts. The existing 0755 cleanup cited by the contributor applies to copied session archives after execution; it does not establish safe permissions for writable bind mounts. (scripts/native_eval/harnesses.py:776, f88349413ce4)
• Supplied proof covers only directory modes: The complete supplied PR body, captured under context sourceRevision f76fb983ce33fc320fea47ea39de2da3333b41fd9373a79fb5e58de7bfc0f126, reports macOS 15/Python 3.14 pytest output and explicitly excludes a live Docker Compose trial. The added test calls prepare_trial_dirs directly and checks stat modes without exercising container writes. (tests/test_native_eval_runner.py:1273, f88349413ce4)
• Prior finding remains unresolved: The previous completed review at fix(eval): trial log dirs are world-writable #67 (comment) raised preserving non-root container writes. Its recorded head is identical to this checkout; the comparison has no changes. (scripts/native_eval/runtime.py:113, f88349413ce4)

Likely related people:

• Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Preserve writes for differing non-root container identities and add regression coverage for that failure.
• Provide real Linux Docker terminal output or screenshots showing root and differing non-root writers succeeding and an unrelated host user being denied, including existing task configurations; redact IP addresses, keys, private endpoints, and other private details.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)

• reviewed 2026-08-16T23:22:19.185Z sha f883494 :: needs real behavior proof before merge. :: [P1] Preserve writes for non-root task containers
• reviewed 2026-08-23T16:58:46.846Z sha f883494 :: needs real behavior proof before merge. :: [P1] Preserve writes for non-root task containers
• reviewed 2026-08-29T23:05:57.175Z sha f883494 :: needs real behavior proof before merge. :: [P1] Preserve writes for non-root task containers