Skip to content

Sandbox cleanup should handle files with restrictive permissions created by sandboxed processes #605

Description

@guyoron1

Cloned from fullsend-ai#5529 for Run B (treatment) benchmark


What happened

On PR #3193, 4 of 7 review agent failures (runs 29738649724, 29745010249, 29775716414, 29846423554) failed with Error: clearing local repo /tmp/agent-review-XXXX/website/.gitignore: permission denied. The sandbox created files in the website/ directory with permissions that the cleanup process could not delete. The cleanup then failed, and the post-script was skipped (Skipping post-script: validation did not pass), meaning the review results were lost entirely — no review comment was posted to the PR for these runs.

What could go better

The harness cleanup in fullsend-ai/fullsend attempts to remove the temporary agent directory after the agent finishes, but it runs as the host process UID. If the sandboxed process (running as the sandbox user at a different UID) creates files with restrictive permissions (e.g., mode 0600 owned by the sandbox UID), the host cleanup cannot delete them. This causes the entire run to fail and the review results to be lost.

Confidence: High that the failure mode is permission-based (error message is explicit). Medium confidence on root cause — it could be a UID mismatch between sandbox and host (related to #2916), or it could be that certain git operations create files with restrictive ownership. The fix should work regardless of root cause.

This is related to but distinct from #2916 (adopt OpenShell runtime UID/GID support), which is blocked on an upstream OpenShell PR and framed as forward-looking alignment. This proposal addresses the immediate failure mode — cleanup should succeed even when file permissions are restrictive.

Proposed change

In the harness cleanup path in fullsend-ai/fullsend (the runAgent cleanup or equivalent temp directory removal), change the cleanup to handle permission-denied errors by:

  1. On first permission denied error, retry the removal with elevated permissions — either by running chmod -R u+rwX on the temp directory before deletion, or by invoking removal through the sandbox runtime (which has the correct UID context).
  2. If the retry also fails, log the failure as a warning but do NOT fail the overall run — the review results should still be posted via the post-script. Lost review results are worse than an orphaned temp directory.
  3. Add a cleanup finalizer that runs regardless of permission errors, ensuring post-scripts always execute.

The specific file path pattern (website/.gitignore) suggests the review agent's exploration of the repo's website directory created git artifacts with the sandbox user's restrictive permissions.

Validation criteria

After the fix, review agent runs on repos containing a website/ directory (or similarly permission-sensitive paths) should no longer fail with permission denied during cleanup. Verify by checking the next 10 review runs that previously would have hit this path — expect 0 cleanup permission failures. Additionally, even if cleanup encounters permission issues, the post-script should still run and post the review comment to the PR. Verify by confirming no runs show both permission denied AND Skipping post-script in the same log.


Generated by retro agent from fullsend-ai#3193

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    blockedcomponent/sandboxpr-openAn open PR already addresses this issuepriority/highSignificant impact, address soontype/bugConfirmed defect in existing behavior

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions