Skip to content

fix: Revert AGENT_SANDBOX env var renames to AGENT_EXECUTOR - #355

Merged
mako-taco merged 1 commit into
mainfrom
jake/apparmor-profiles
Jul 31, 2026
Merged

fix: Revert AGENT_SANDBOX env var renames to AGENT_EXECUTOR#355
mako-taco merged 1 commit into
mainfrom
jake/apparmor-profiles

Conversation

@mako-taco

Copy link
Copy Markdown
Contributor

Summary

Reverts the incorrect AGENT_SANDBOX_*AGENT_EXECUTOR_* env var renames that were included in #353. The application expects AGENT_SANDBOX_* names.

Changes:

  • AGENT_EXECUTOR_POSTGRES_URLAGENT_SANDBOX_POSTGRES_URL
  • AGENT_EXECUTOR_ROLEAGENT_SANDBOX_ROLE
  • AGENT_EXECUTOR_POSTGRES_SCHEMAAGENT_SANDBOX_POSTGRES_SCHEMA
  • AGENT_EXECUTOR_POSTGRES_POOL_MAXAGENT_SANDBOX_POSTGRES_POOL_MAX
  • AGENT_EXECUTOR_JWT_PUBLIC_KEYAGENT_SANDBOX_JWT_PUBLIC_KEY
  • AGENT_EXECUTOR_ENCRYPTION_KEYAGENT_SANDBOX_ENCRYPTION_KEY

Made with Cursor

@mertbozfakioglu
mertbozfakioglu self-requested a review July 31, 2026 19:51
@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

The js-executor render-gate mismatch must be fixed before merging because a supported configuration leaves all js-executor pods blocked indefinitely.

js-executor can render its new marker wait while the only DaemonSet that creates that marker is omitted, and the CI bootstrap also executes a mutable remote installer with root privileges.

Files Needing Attention: charts/retool/templates/deployment_js_executor.yaml, charts/retool/templates/apparmor_nsjail_daemonset.yaml, .github/test-apparmor.sh

Security Review

The new integration test executes an unpinned remote k3s installer as root; pin and verify the installer before execution.

Important Files Changed

Filename Overview
charts/retool/templates/deployment_js_executor.yaml Adds AppArmor selection and readiness waiting, but its gate can render the waiter without the required installer DaemonSet.
charts/retool/templates/apparmor_nsjail_daemonset.yaml Adds the privileged per-node profile installer, whose workflows-only render gate is incompatible with independently enabled js-executor.
charts/retool/templates/deployment_agent_sandbox.yaml Adds an AppArmor readiness gate and profile selection to sandbox jobs plus writable temporary volumes for controller and proxy.
charts/retool/templates/agent_sandbox_seccomp.yaml Extends the existing sandbox node installer to load the optional AppArmor profile and publish a per-node readiness marker.
.github/test-apparmor.sh Adds end-to-end profile verification, with a non-blocking supply-chain concern from executing the mutable k3s installer as root.
.github/workflows/ci.yaml Adds the AppArmor integration job as a required validation dependency.
charts/retool/values.yaml Adds disabled-by-default AppArmor installer options for executor and agent-sandbox workloads.

Reviews (1): Last reviewed commit: "Revert AGENT_SANDBOX_* -> AGENT_EXECUTOR..." | Re-trigger Greptile

Comment on lines +81 to +108
{{- if .Values.codeExecutor.appArmorProfileInstaller }}
- name: wait-for-apparmor
image: busybox:1.37.0@sha256:b3255e7dfbcd10cb367af0d409747d511aeb66dfac98cf30e97e87e4207dd76f
securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: true
capabilities:
drop: ["ALL"]
resources:
requests:
cpu: 1m
memory: 4Mi
limits:
cpu: 10m
memory: 16Mi
command:
- /bin/sh
- -c
- |
until [ -f /host-run/retool-executor.loaded ]; do
echo "Waiting for AppArmor profile retool-executor to be loaded by DaemonSet..."
sleep 2
done
echo "AppArmor profile retool-executor loaded"
volumeMounts:
- name: host-run
mountPath: /host-run
readOnly: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 AppArmor installer gate mismatch

When rr.jsExecutor.enabled=true, codeExecutor.appArmorProfileInstaller=true, and workflows remain disabled, this init container waits for retool-executor.loaded even though the workflows-gated DaemonSet that creates it is not rendered, leaving every js-executor pod stuck in initialization.

Comment thread .github/test-apparmor.sh
echo " k3s is already running, skipping install."
else
echo " Installing k3s..."
curl -sfL https://get.k3s.io | INSTALL_K3S_EXEC="--disable=traefik" sh -

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 security Mutable root-level installer execution

The new CI job reaches this unversioned curl | sh path on each fresh runner and executes the downloaded response as root, making CI behavior dependent on mutable remote content and granting a compromised response full control of the runner.

How this was verified: The workflow invokes this script with sudo, and the fresh-runner branch pipes https://get.k3s.io directly into sh without integrity verification.

The env var names should stay as AGENT_SANDBOX_* to match what the
application expects. The rename to AGENT_EXECUTOR_* was incorrect.

Co-authored-by: Cursor <cursoragent@cursor.com>
@mako-taco
mako-taco force-pushed the jake/apparmor-profiles branch from 12daccf to f9caad0 Compare July 31, 2026 19:53

mako-taco commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • Jul 31, 7:54 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 31, 7:54 PM UTC: @mako-taco merged this pull request with Graphite.

@mako-taco
mako-taco merged commit 5f27ad5 into main Jul 31, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants