Skip to content

Enforce execution profile sandbox settings (prevent manifest overrides)#13

Merged
homerquan merged 1 commit into
mainfrom
codex/fix-execution-profiles-vulnerability
May 18, 2026
Merged

Enforce execution profile sandbox settings (prevent manifest overrides)#13
homerquan merged 1 commit into
mainfrom
codex/fix-execution-profiles-vulnerability

Conversation

@homerquan

Copy link
Copy Markdown
Collaborator

Motivation

  • Execution profiles were being treated as placement-only selectors while manifest-provided sandbox/OpenShell settings could override operator-defined profile security fields, enabling attackers to hijack image, policy, ssh_key, workspace, and pool settings.
  • The change makes profiles the authoritative source for OpenShell/security-sensitive fields when a manifest selects an execution_profile to preserve operator intent and placement semantics.

Description

  • Change apply_to_config/1 in lib/mirror_neuron/execution/profile.ex to drop manifest-supplied OpenShell/secure keys before merging the trusted profile config so profile-owned keys take precedence.
  • Add @profile_local_cli_path_keys and node-side expansion of profile-local CLI paths (policy and ssh_key) so relative profile paths are resolved on the runtime node instead of against a submitted payload bundle.
  • Keep non-OpenShell manifest config (e.g. unrelated runner settings) intact by only dropping keys listed in @openshell_keys from the manifest portion prior to merging.
  • Add regression unit coverage in tests/unit/execution_profile_test.exs validating that manifest-supplied sandbox values cannot override profile-controlled from/policy/ssh_key/pool/pool_slots/reuse_shared_sandbox/persistent_workspace/sandbox_upload_path, and update README to document the profile immutability semantics and path resolution behavior.

Testing

  • Ran mix format and git diff --check, both completed with no formatting or diff-check errors.
  • Performed an inline Elixir runtime assertion using elixir -r lib/mirror_neuron/execution/profile.ex which passed and validated that profile values override manifest-supplied values as intended.
  • Attempted mix test tests/unit/execution_profile_test.exs but dependency fetching failed due to environment network restrictions (Hex/GitHub fetch returned HTTP CONNECT 403), so full test suite could not be executed here.
  • Attempted to install Hex and fetch deps (mix local.hex, mix archive.install ..., mix deps.get) but these operations were blocked by the same network policy and returned 403 errors.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e37bfc43a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

defp manifest_config_for_profile(config) do
config
|> stringify_map()
|> Map.drop(@openshell_keys)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Drop manifest-supplied sandbox CLI with profile

When an untrusted manifest selects an execution_profile and also sets sandbox_cli, this filtering keeps that key because it is not in @openshell_keys; OpenShell later uses Map.get(config, "sandbox_cli", ...) as the executable passed to System.cmd. That leaves the manifest able to choose the local OpenShell binary even though the profile is now supposed to own sandbox/security settings, so add this invocation control key to the profile-owned/drop set or otherwise prevent manifest override.

Useful? React with 👍 / 👎.

@homerquan homerquan merged commit 76afea1 into main May 18, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant