[feat][PLAT-1416] Add AppArmor profile installers for code-executor and agent-sandbox - #353
Merged
Conversation
…nd agent-sandbox On Ubuntu 24.04+ with kernel.apparmor_restrict_unprivileged_userns=1, the default "unconfined" AppArmor annotation prevents nsjail and gVisor/pasta from creating unprivileged user namespaces. This adds opt-in AppArmor profile installers that load confined profiles with an explicit `userns,` rule, allowing sandboxing to work without requiring the customer to set unconfined (which some security-conscious customers like P&G cannot accept). For code-executor / js-executor (nsjail): - New DaemonSet + ConfigMap load usr.bin.nsjail and retool-executor profiles on each node via nsenter + apparmor_parser - Gated by codeExecutor.appArmorProfileInstaller (default false) - Switches the annotation from "unconfined" to "localhost/retool-executor" For agent-sandbox (gVisor/pasta): - Extends the existing node-installer DaemonSet with an install-apparmor init container + ConfigMap for the retool-agent-sandbox profile - Gated by rr.agentSandbox.appArmorProfileInstaller (default false) - Switches the job template from Unconfined to Localhost profile CI: kubeconform option files for schema validation, plus a ct-install values file that exercises the nsjail AppArmor path end-to-end in kind. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
| Filename | Overview |
|---|---|
| charts/retool/templates/apparmor_nsjail_daemonset.yaml | Loads the nsjail and executor profiles before atomically signaling node-local readiness; the previously reported parser and marker-lifecycle issues are addressed. |
| charts/retool/templates/agent_sandbox_seccomp.yaml | Extends the node installer to load the agent-sandbox profile and publishes readiness only after successful parsing. |
| charts/retool/templates/deployment_code_executor.yaml | Selects the Localhost profile when enabled and waits for confirmed node-local installation before starting the protected container. |
| charts/retool/templates/deployment_js_executor.yaml | Mirrors the code-executor profile selection and readiness coordination for js-executor. |
| charts/retool/templates/deployment_agent_sandbox.yaml | Adds profile readiness coordination and a Localhost AppArmor security context to generated sandbox Jobs. |
| charts/retool/templates/apparmor_nsjail_configmap.yaml | Defines the nsjail and retool-executor AppArmor profiles with explicit user-namespace permission. |
| charts/retool/templates/apparmor_agent_sandbox_configmap.yaml | Defines the confined agent-sandbox profile with explicit user-namespace permission. |
| .github/test-apparmor.sh | Exercises profile loading, profile attachment, and user-namespace creation on an AppArmor-enabled test host. |
| .github/workflows/ci.yaml | Adds the AppArmor integration test as a prerequisite of PR validation. |
Reviews (8): Last reviewed commit: "fix: wait for k3s node registration befo..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The root values.yaml was missing the AppArmor settings that were added to charts/retool/values.yaml, causing the "Ensure values.yaml files match" CI check to fail. Also bumps the chart version from 6.11.12 to 6.11.13 so ct lint passes the version-bump check. Co-authored-by: Cursor <cursoragent@cursor.com>
| # "unconfined" to "localhost/retool-executor", which satisfies Ubuntu 24.04+'s | ||
| # kernel.apparmor_restrict_unprivileged_userns=1 restriction by providing an | ||
| # explicit userns rule. Requires useSeccompProfile: true to take effect. | ||
| appArmorProfileInstaller: false |
Contributor
There was a problem hiding this comment.
we probably want this on by default
Contributor
Author
There was a problem hiding this comment.
I thought it would be wasteful to run the daemonset if we were not deploying on ubuntu
Contributor
There was a problem hiding this comment.
especially since this installer needs to be run privileged
| profile retool-agent-sandbox flags=(attach_disconnected,mediate_deleted) { | ||
| #include <abstractions/base> | ||
|
|
||
| network, |
Contributor
There was a problem hiding this comment.
I love the very explicit app armor profile but it'd need thorough manual testing
Contributor
There was a problem hiding this comment.
maybe we can first do this change for our cloud and once confirm that the allow/deny list here works well then push this change
The kind clusters used by ct-install don't have AppArmor enabled, so pods requesting localhost/ profiles fail with "Cannot enforce AppArmor: AppArmor is not enabled on the host" and the Helm install times out. AppArmor template rendering is already validated by kubeconform via the option files (test-apparmor-nsjail-option.yaml and test-apparmor-agent-sandbox-option.yaml). Actual cluster testing requires an AppArmor-enabled host, which isn't available in CI. Co-authored-by: Cursor <cursoragent@cursor.com>
- Fail installer init containers when apparmor_parser is missing instead of silently skipping, so pods aren't left requiring profiles that were never loaded. - Add wait-for-apparmor init containers to code-executor, js-executor, and agent-sandbox workloads so they block until the DaemonSet has installed the required profile on the node. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@greptile review |
…e: 3/5) Installers now write a .loaded sentinel file only after apparmor_parser successfully loads the profile into the kernel. Wait init containers check the sentinel instead of the profile file, closing the race where a workload could proceed before the profile was actually loaded. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@greptile review |
…: 3/5) Sentinel files now live under /run/retool/apparmor/ (a tmpfs cleared on every boot) instead of /etc/apparmor.d/. This prevents stale markers from surviving node reboots while the kernel's loaded profiles are cleared, closing the last race between installer DaemonSets and workload pods. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@greptile review |
The test-apparmor CI check was failing because `helm dependency build` couldn't fetch the postgresql chart without the bitnami repo configured. Other CI jobs use chart-testing which handles this via ct.yaml, but the apparmor test calls helm directly. Also includes env var renames (AGENT_SANDBOX -> AGENT_EXECUTOR) and adds /tmp emptyDir volume mounts to controller/proxy pods. Co-authored-by: Cursor <cursoragent@cursor.com>
kubectl get nodes returns success even with an empty node list, causing the wait loop to break prematurely. Check that at least one node appears in the output before proceeding to kubectl wait --for=condition=Ready. Co-authored-by: Cursor <cursoragent@cursor.com>
mako-taco
marked this pull request as ready for review
July 29, 2026 22:58
Contributor
Author
|
@tryretool/security we bug bashed this and things look functional, but we're definitely not experts on apparmor. would you mind doing a look over? |
Contributor
|
Just wanted to make sure that when you tested it, you tested it with kernel.apparmor_restrict_unprivileged_userns=1 set on the host machine |
| - name: DAEMONSET_NAME | ||
| value: {{ include "retool.agentSandbox.name" . }}-image-prepuller | ||
| {{- if $as.jwtPublicKey }} | ||
| - name: AGENT_SANDBOX_JWT_PUBLIC_KEY |
Contributor
There was a problem hiding this comment.
these changes are really surprising to me. I'd maybe put them in a separate PR and get Matt's approval
| - name: install | ||
| image: {{ .Values.codeExecutor.appArmorInstallerImage | default "busybox:1.37" }} | ||
| securityContext: | ||
| privileged: true |
Contributor
There was a problem hiding this comment.
I don't love that this need privileges. Is this why you had set this appArmor path off by default?
mertbozfakioglu
approved these changes
Jul 31, 2026
Contributor
Author
Merge activity
|
mako-taco
added a commit
that referenced
this pull request
Jul 31, 2026
## 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_URL` → `AGENT_SANDBOX_POSTGRES_URL` - `AGENT_EXECUTOR_ROLE` → `AGENT_SANDBOX_ROLE` - `AGENT_EXECUTOR_POSTGRES_SCHEMA` → `AGENT_SANDBOX_POSTGRES_SCHEMA` - `AGENT_EXECUTOR_POSTGRES_POOL_MAX` → `AGENT_SANDBOX_POSTGRES_POOL_MAX` - `AGENT_EXECUTOR_JWT_PUBLIC_KEY` → `AGENT_SANDBOX_JWT_PUBLIC_KEY` - `AGENT_EXECUTOR_ENCRYPTION_KEY` → `AGENT_SANDBOX_ENCRYPTION_KEY` Made with [Cursor](https://cursor.com)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Ubuntu 24.04+ with
kernel.apparmor_restrict_unprivileged_userns=1, the defaultunconfinedAppArmor annotation prevents nsjail (code-executor/js-executor) and gVisor/pasta (agent-sandbox) from creating unprivileged user namespaces. Some customers (e.g. P&G) cannot acceptunconfinedprofiles due to security governance.This adds opt-in AppArmor profile installers that load confined profiles with an explicit
userns,rule, so sandboxing works without requiringunconfined.code-executor / js-executor (nsjail):
apparmor_nsjail_daemonset.yaml+apparmor_nsjail_configmap.yaml— DaemonSet loadsusr.bin.nsjailandretool-executorprofiles on each node viansenter -t 1 -m -- apparmor_parser -rcodeExecutor.appArmorProfileInstaller: true(defaultfalse)unconfinedtolocalhost/retool-executorcodeExecutor.useSeccompProfile: true(the annotation block is gated on it)agent-sandbox (gVisor/pasta):
apparmor_agent_sandbox_configmap.yamlwith theretool-agent-sandboxprofileagent_sandbox_seccomp.yamlnode-installer DaemonSet with aninstall-apparmorinit containerrr.agentSandbox.appArmorProfileInstaller: true(defaultfalse)appArmorProfilefromUnconfinedto{"type": "Localhost", "localhostProfile": "retool-agent-sandbox"}CI coverage:
test-apparmor-nsjail-option.yamlandtest-apparmor-agent-sandbox-option.yamlfor kubeconform schema validationtest-apparmor-install-values.yamlfor ct-install startup testing in kindCloses PLAT-1416, PLAT-1426.
Test Plan
kernel.apparmor_restrict_unprivileged_userns=1:retool-executor (enforce)andretool-agent-sandbox (enforce)confirmed in/sys/kernel/security/apparmor/profilesretool-executor (enforce)and nsjail successfully creates user/mount/PID namespacesretool-agent-sandbox (enforce)successfully creates user/mount/PID namespacesunconfined/Unconfined— no behavioral change for existing usersMade with Cursor