Skip to content

fix(openshell): check sandbox policy settings at controller startup - #3499

Merged
freeqaz merged 2 commits into
mainfrom
fix-fe3-openshell-policy
Oct 24, 2026
Merged

freeqaz merged 2 commits into
mainfrom
fix-fe3-openshell-policy

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The OpenShell Sandbox Driver accepted several settings at controller startup that fail only when a dedicated deploy creates its Sandbox, after the deploy has stopped its predecessors and written the runtime profile and provider to the Gateway:

  • policy.process.runAsUser / runAsGroup were checked for being nonempty only. "root", "0" or "users" loaded, and the pinned Gateway's CreateSandbox then refused them (validate_policy_safety -> is_valid_sandbox_identity: sandbox or a numeric ID from 1 to u32::MAX-1) with INVALID_ARGUMENT.
  • policy.filesystem was never read at startup. Relative paths, /, and a read-write grant on a read-only sandboxDataMount were refused only inside sandboxSpec while provisioning; /usr/../etc and // passed that check too and were refused by the Gateway (validate_filesystem_paths). An unknown key under policy.filesystem (for example readwrite) was silently ignored.
  • kubernetes.agentResources.requests / limits that were not objects ("1", ["x"]) threw only in sandboxSpec.

Findings 2331, 2332, 2333.

Change

  • validateOptions runs a new validatePolicySettings: process identities use the Gateway's rule; filesystem paths use one shared rule (isFilesystemPolicyPath), which provisioning's validateFilesystemPolicyPath now also uses, so it refuses .. segments and slash-only paths as well; a read-only data mount listed in readWrite is refused. Each refusal is a ConfigurationValidationError that names the setting.
  • The configuration schema requires policy.process with both identities (an omitted block is a REQUIRED refusal, not a crash) and declares policy.filesystem (strict: includeWorkdir boolean, readOnly/readWrite string arrays, no other keys) and kubernetes.agentResources.requests/limits as objects.
  • Breaking-change notice in docs/guides/deploy/breaking-changes.md. No values a working Installation can use are refused: each refused value already failed every dedicated deploy.

Verification

  • New tests in tests/integration/sandbox-driver-startup.test.mjs ("startup refuses OpenShell process identities the gateway refuses", "... filesystem paths before any Sandbox is created", "... Agent resources that are not objects") fail on main (missing rejection / wrong path) and pass with the fix; control cases load sandbox and 4294967294.
  • Control cases also load +5 and 007, which the Gateway's u32 parser accepts, and a writable data mount in readWrite; a NUL path and an omitted policy.process / runAsGroup are refused.
  • Whole file: 118 pass, 0 fail, 1 skipped (Node 24); pinned 20/20 at 2 CPUs on a remote test VM on the final head. Also green locally: runtime-logs-sandbox, workspace-setup-api, static-egress, protected-driver-registration, repository-credentials-admission.
  • Internal review: approve-with-nits; applied (required process schema, extra identity/NUL/control cases, notice wording). Not mirrored: the Gateway's 256-path and 4096-byte limits. The driver reference page is at its word budget, so the accepted identity values are stated in the startup error and the notice rather than there.
  • tsc --build tsconfig.json clean; eslint and prettier clean on touched files.
  • Gateway rules read from the pinned OpenShell source (crates/openshell-policy/src/lib.rs is_valid_sandbox_identity, validate_filesystem_paths; crates/openshell-server/src/grpc/sandbox.rs CreateSandbox).

Risks

The OpenShell Gateway refuses a process identity other than sandbox or
1..4294967294 and relative, root or traversing filesystem paths only in
CreateSandbox, and the driver refused non-object Agent resources and a
read-write grant on a read-only data mount only while building the Sandbox
spec. A dedicated deploy reaches both after its predecessors have stopped.

Startup now applies the same rules and names the setting: process identities,
filesystem paths (one shared rule with provisioning), the read-only data mount,
unknown filesystem keys and the Agent resource shapes. Findings 2331-2333.
@freeqaz
freeqaz requested a review from a team as a code owner October 24, 2026 17:58
@clawsweeper

clawsweeper Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review in progress

ClawSweeper is reviewing this revision. This supersedes any previous blocked status.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 24, 2026
…pellings

Review follow-ups: policy.process and its two identities are required by the
schema, so an omitted block is a REQUIRED refusal instead of a TypeError; the
tests pin the '+5' and '007' spellings the gateway's u32 parser accepts, a NUL
path and a writable data mount in readWrite; the notice no longer claims every
refused value failed deploys (a slash-only readOnly path did not).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant