fix(onboard): require journal for not-ready repair - #8027
Conversation
Signed-off-by: Ho Lim <subhoya@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe onboarding sandbox flow removes recorded-sandbox repair APIs and dependencies. Not-ready sandboxes now require journal-bound recreation. Tests provide gateway authority and recreate observations, and verify fail-closed behavior. ChangesSandbox recreation flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
1 terminology difference from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
2 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. Since last review: 0 prior items resolved · 0 still apply · 0 new items found 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
|
@cv @NVIDIA/nemoclaw-maintainer Could you please help with the protected E2E disposition for exact head fcacdc3?
I cannot authorize or repair that trusted-run path from the fork. I will keep this head unchanged so checks are not reset again. Please either authorize/retrigger the repository-supported E2E path for this exact head, identify any remaining code or evidence blocker I can address, or mark the PR superseded if its outcome is no longer wanted. |
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Reviewed current head fcacdc3. The change removes the unjournaled direct sandbox-delete and registry-removal fallback while preserving the journal-owned recreate and repair-event path; missing journal authority now refuses before deletion. The required E2E failure is a trusted-verdict coordination timeout rather than a product test failure. I found no blocking correctness or security defect.
Signed-off-by: Ho Lim <subhoya@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agents/hermes/Dockerfile (1)
1021-1027: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftSecurity Misconfiguration (CWE-732): Incorrect Permission Assignment for Critical Resource
Reachability: Internal · Exploitability: Moderate
Replace the Darwin-wide permission change with scoped access.
With
NEMOCLAW_DARWIN_VM_COMPAT=1, the block changes/sandbox/.hermes/.envandconfig.yamlfrom0640to0666, and makes state directories world-writable. Non-root startup then rejects the world-writable.envbefore runtime repair. Use UID/group-scoped permissions and add negative-path access tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/hermes/Dockerfile` around lines 1021 - 1027, Update the NEMOCLAW_DARWIN_VM_COMPAT permission block in the Dockerfile to avoid recursive/world-writable access, preserving restrictive modes for .hermes/.env and config.yaml while granting only the required runtime UID/group access to NemoClaw state paths. Add negative-path tests that verify world-writable .env/configuration files and state directories are rejected while the intended scoped access succeeds.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@agents/hermes/Dockerfile`:
- Around line 1021-1027: Update the NEMOCLAW_DARWIN_VM_COMPAT permission block
in the Dockerfile to avoid recursive/world-writable access, preserving
restrictive modes for .hermes/.env and config.yaml while granting only the
required runtime UID/group access to NemoClaw state paths. Add negative-path
tests that verify world-writable .env/configuration files and state directories
are rejected while the intended scoped access succeeds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2cac9bd6-5e3b-4a24-bea6-8829fd2f1854
📒 Files selected for processing (2)
agents/hermes/Dockerfileci/source-architecture-budget.json
wscurran
added
area: onboarding
Signed-off-by: Ho Lim <subhoya@gmail.com>
Summary
Contributes to #6492. This is the final legacy-fallback removal after #7788 journal-bound rebuild and re-onboard adoption.
Validation
npm run build:clinpm run typecheck:clinpm run check:diffreaches an unrelated current-main repository budget failure:src/lib/core/ports.tsfan-in is 85 while its stale limit is 86. This change does not touch that file or dependency boundary.Summary by CodeRabbit
Bug Fixes
Refactor
Signed-off-by: Ho Lim subhoya@gmail.com