feat(shell): add zero-config default like the database worker - #320
Conversation
The shell worker required a config file to boot: ShellConfig::default() is intentionally unjailed (host_root: None) and rejected by build_runtime, and register_config never seeded a built-in value, so with no --config and nothing stored the worker failed closed at boot. Add ShellConfig::seed_default(), a bootable permissive dev default (jailed to /tmp, env forwarded, open exec with a catastrophic-only denylist) that mirrors the shipped config.yaml, drift-guarded by a unit test. register_config seeds it as initial_value on first registration when no --config seed is given and nothing is stored, so the worker boots with no config file at all (database parity). Default::default() stays unjailed so a partial operator config still fails closed, and fetch_config still returns it on a null stored value: boot fails closed and a hot-reload keeps last-good rather than silently widening the live jail to the /tmp seed. Claude-Session: https://claude.ai/code/session_01YCgHHh8hLBXy9v1zKjGLtK
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 53 minutes and 27 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a ChangesShell worker zero-config default seeding
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
skill-check — worker0 verified, 25 skipped (no docs/).
Four for four. Nicely done. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shell/src/config.rs (1)
179-191: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign
seed_default()docs with the actual null-fallback behavior.Line 181 currently says
seed_default()is the runtime fallback fornullstored config, but runtime null fallback is actuallyShellConfig::default()(fail-closed). Please update the comment to avoid security-semantics confusion.📝 Suggested doc-only fix
- /// registration and used as the runtime fallback when the stored value is - /// null, so the worker boots with no config file at all (database-style - /// zero-config). This is deliberately NOT `Default::default()` — that is + /// registration when no value is stored yet, so the worker can boot with + /// no config file at all (database-style zero-config). Runtime fallback on + /// a stored `null` remains `Default::default()` (intentionally invalid) so + /// boot fails closed. This is deliberately NOT `Default::default()` — that is🤖 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 `@shell/src/config.rs` around lines 179 - 191, The documentation comment for the seed_default() method incorrectly states that it is used as the runtime fallback when the stored value is null, but the actual runtime null fallback is ShellConfig::default() (fail-closed). Update the comment to clarify that seed_default() is used only as the initial value on first registration, and explicitly note that the runtime fallback for null stored config is ShellConfig::default(), not seed_default(), to avoid security-semantics confusion.
🤖 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.
Nitpick comments:
In `@shell/src/config.rs`:
- Around line 179-191: The documentation comment for the seed_default() method
incorrectly states that it is used as the runtime fallback when the stored value
is null, but the actual runtime null fallback is ShellConfig::default()
(fail-closed). Update the comment to clarify that seed_default() is used only as
the initial value on first registration, and explicitly note that the runtime
fallback for null stored config is ShellConfig::default(), not seed_default(),
to avoid security-semantics confusion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90ff6d9b-8a1e-48b3-8353-fe0f463062e8
📒 Files selected for processing (5)
shell/ARCHITECTURE.mdshell/README.mdshell/src/config.rsshell/src/configuration.rsshell/src/main.rs
2 participants
What
Gives the
shellworker a zero-config default so it boots with no config file at all, matching thedatabaseworker's pattern.Before this, the shell worker could not boot config-less:
ShellConfig::default()is intentionally unjailed (host_root: None) and rejected bybuild_runtime, andregister_confignever seeded a built-in value. With no--configand nothing stored, boot failed closed.How
config.rs— addShellConfig::seed_default(): a bootable permissive dev default (jailed to/tmp,inherit_env: true, open exec with a catastrophic-only denylist). It mirrors the shippedconfig.yamlexactly, drift-guarded byseed_default_matches_shipped_config_yaml.configuration.rs—register_configseedsseed_default()asinitial_valueon first registration when no--configseed is given and nothing is stored yet (should_seed_default_value), validated viabuild_runtimebefore persisting.prepare_config_accepts_seed_default.Safety
ShellConfig::default()stays unchanged (unjailed) so a partial operator config that omits the jail still fails closed.fetch_configstill returnsdefault()on a null stored value, so:/tmpseed.Note: the zero-config default carries the existing shipped
config.yamlposture (/tmpjail,inherit_env: true, open exec) — a config-less boot now behaves exactly likecargo rundid before. Tightening that default (e.g.inherit_env: false) would be a separate decision.Verification
cargo buildclean,cargo clippyclean, 243 lib tests pass. Reviewed via/review(boot/reload paths traced, independent adversarial pass — no security regressions).https://claude.ai/code/session_01YCgHHh8hLBXy9v1zKjGLtK
Summary by CodeRabbit
Release Notes
New Features
Documentation
Improvements