fix(wasm-helpers): unique sandbox exec capture id per entity (ARN-401) - #468
Open
rita-aga wants to merge 1 commit into
Open
fix(wasm-helpers): unique sandbox exec capture id per entity (ARN-401)#468rita-aga wants to merge 1 commit into
rita-aga wants to merge 1 commit into
Conversation
…N-401)
The sandbox exec temp files /tmp/.paw-{out,err,rc}-<id> collided between
concurrent execs: unique_run_id() used a process-local AtomicU32 starting at
0, but every trigger dispatch is a fresh WASM instance, so the counter reset
each time and every exec reused /tmp/.paw-out-00000000. Two concurrent execs
on one sandbox then crossed stdout (and raced the cleanup delete) — a
ReleaseRun rollback misread a concurrent health-probe's 502 output as its
git-revert result.
Scope the id to the calling entity (ctx.entity_id, sanitized) plus the host
clock and a per-instance counter, so two different entities' execs never
collide. Pure helper capture_run_id() covered by unit tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
Comment on lines
+738
to
+744
| let entity: String = entity_id | ||
| .chars() | ||
| .map(|c| { | ||
| if c.is_ascii_alphanumeric() || c == '-' || c == '_' { | ||
| c | ||
| } else { | ||
| '_' |
There was a problem hiding this comment.
If distinct entity IDs such as a/b and a?b execute concurrently on the same sandbox during the same clock millisecond, this mapping gives both the same sanitized token and capture paths, allowing their output to cross or one invocation to delete the other's files.
Prompt To Fix With AI
This is a comment left during a code review.
Path: os-apps/paw-agent/wasm/wasm-helpers/src/sandbox.rs
Line: 738-744
Comment:
**Lossy entity-ID sanitization**
If distinct entity IDs such as `a/b` and `a?b` execute concurrently on the same sandbox during the same clock millisecond, this mapping gives both the same sanitized token and capture paths, allowing their output to cross or one invocation to delete the other's files.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The sandbox exec output-capture temp files
/tmp/.paw-{out,err,rc}-<id>collided between concurrent execs on one sandbox.unique_run_id()used only a process-localAtomicU32starting at 0 — but every trigger dispatch is a fresh WASM instance, so the counter reset to 0 each time and every exec reused/tmp/.paw-out-00000000. Two concurrent execs then crossed each other's stdout and raced the cleanup delete.Live impact (ARN-401): a governed
ReleaseRunrollback ran concurrently with a healthy release's health probe on the same computer; the rollback'ssandbox_execreturned the probe's502 __HTTP_STATUSbody instead of thegit revertoutput, so a successful revert was reported as Failed.Fix
Scope the capture id to the calling entity:
capture_run_id(ctx.entity_id, host_clock, per-instance seq). Two different entities' execs never collide (differententity_id); the clock + counter separate repeats from one entity/instance. The entity id is sanitized to a filename-safe token (can't escape/tmp/.paw-*or break the shell redirect); empty falls back toexec.Tests
capture_run_idunit tests: entity-scoped (the two-concurrent-entities case), repeat separation, and shell/path sanitization. 3/3 pass.Deploy note
wasm-helpersis a shared path dep; the exec path iscomputer_exec(paw-compute). This PR fixes the source on main;computer_execis rebuilt against it and republished to Genesis separately. Pairs with ARN-397 (per-repo release serialization) which prevents the overlap at the workflow level.Linear: ARN-401
🤖 Generated with Claude Code
https://claude.ai/code/session_01PZkYwk1gmU6GPKRuQsSLda
Greptile Summary
The PR scopes sandbox-exec capture filenames using the calling entity, host time, and a per-instance sequence to prevent concurrent executions from sharing output files.
Confidence Score: 4/5
The PR appears safe to merge, with a non-blocking uniqueness gap for distinct entity IDs that sanitize to the same filename token.
The new entity, clock, and sequence tuple addresses the reported common collision, but replacing every unsafe character with
_means distinct punctuated entity IDs can still converge on the same capture paths under simultaneous dispatch.Files Needing Attention: os-apps/paw-agent/wasm/wasm-helpers/src/sandbox.rs
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Entity ID] --> B[Replace unsafe characters with underscore] B --> C[Append host milliseconds and instance sequence] C --> D[Build out, err, and rc paths] D --> E[Execute and capture command output] A1[Distinct ID a/b] --> B A2[Distinct ID a?b] --> B B --> F[Possible identical sanitized token] F --> CPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(wasm-helpers): make sandbox exec cap..." | Re-trigger Greptile