Implement best practices for running untrusted code in containers (resolves #122)#2290
Conversation
Greptile SummaryThis PR hardens the container sandbox for untrusted action workloads by switching
Confidence Score: 3/5Safe to merge only after confirming that the production S3 endpoint is reachable from a Docker bridge-network container; if it is an internal compose service, artifact uploads will fail in production. The user-action hardening (CapDrop ALL, no-new-privileges, bridge network) is correct and well-motivated. The concern is that launchArtifactUploadContainer — a trusted infrastructure container whose job is to persist action results — is now placed on the same bridge network as untrusted containers. If S3_ENDPOINT is an internal Docker-compose service (common in self-hosted deployments), the bridge network isolates the uploader from S3 and every artifact upload silently fails in production. queueConsumer/src/actions/services/docker-daemon.service.ts — specifically the network mode applied to launchArtifactUploadContainer and how S3_ENDPOINT is resolved in production.
|
| Filename | Overview |
|---|---|
| queueConsumer/src/actions/helper/container-configs.ts | Tightens security defaults: CapDrop to ['ALL'] (functionally equivalent to the previous explicit 14-cap list, but more future-proof), SecurityOpt corrected to 'no-new-privileges:true', and NetworkMode changed from 'host' to 'bridge'. |
| queueConsumer/src/actions/services/docker-daemon.service.ts | Applies the new bridge network mode conditionally (host in DEV, bridge in production) at two container-creation sites. The artifact uploader — a trusted infrastructure container that must reach S3 — is now also placed on the bridge network in production, which may prevent it from reaching internal S3 endpoints. |
| docs/usage/actions/write-actions.md | Documentation updated to surface the new security constraints (network isolation, dropped capabilities, no-new-privileges). Content is accurate and clear. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant QC as QueueConsumer
participant DD as DockerDaemon
participant UA as User Action Container
participant AU as Artifact Uploader Container
participant S3 as S3 Endpoint
QC->>DD: startContainer(options)
DD->>UA: "create (CapDrop=ALL, no-new-privs, NetworkMode=bridge[prod]/host[dev])"
UA-->>DD: "action runs & writes to /out volume"
DD->>QC: container done
QC->>DD: launchArtifactUploadContainer(actionUuid, runnerId)
DD->>AU: "create (CapDrop=ALL, no-new-privs, NetworkMode=bridge[prod]/host[dev])"
Note over AU,S3: bridge mode in prod — S3 must be publicly routable
AU->>S3: upload /out artifacts
S3-->>AU: 200 OK
AU-->>DD: ARTIFACT_METADATA logged
DD->>QC: return metadata
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant QC as QueueConsumer
participant DD as DockerDaemon
participant UA as User Action Container
participant AU as Artifact Uploader Container
participant S3 as S3 Endpoint
QC->>DD: startContainer(options)
DD->>UA: "create (CapDrop=ALL, no-new-privs, NetworkMode=bridge[prod]/host[dev])"
UA-->>DD: "action runs & writes to /out volume"
DD->>QC: container done
QC->>DD: launchArtifactUploadContainer(actionUuid, runnerId)
DD->>AU: "create (CapDrop=ALL, no-new-privs, NetworkMode=bridge[prod]/host[dev])"
Note over AU,S3: bridge mode in prod — S3 must be publicly routable
AU->>S3: upload /out artifacts
S3-->>AU: 200 OK
AU-->>DD: ARTIFACT_METADATA logged
DD->>QC: return metadata
Comments Outside Diff (2)
-
queueConsumer/src/actions/services/docker-daemon.service.ts, line 590-597 (link)Artifact uploader bridge network may sever S3 connectivity
The artifact uploader is a trusted internal container that must reach
S3_ENDPOINTto persist action results. Before this PR it ran with host networking in all environments; now it uses bridge mode in production. In bridge mode the container has a private network namespace: bothlocalhostand127.0.0.1resolve to the container's own loopback, not the host. IfS3_ENDPOINTis configured as an internal Docker-compose service name (e.g. a private MinIO instance on a compose network) that name will not be resolvable from the default bridge network, causing every artifact upload to fail silently in production. The'localhost' → '127.0.0.1'substitution at line 583 does not address this gap — it predates bridge mode and only made sense under host networking.Prompt To Fix With AI
This is a comment left during a code review. Path: queueConsumer/src/actions/services/docker-daemon.service.ts Line: 590-597 Comment: **Artifact uploader bridge network may sever S3 connectivity** The artifact uploader is a trusted internal container that must reach `S3_ENDPOINT` to persist action results. Before this PR it ran with host networking in all environments; now it uses bridge mode in production. In bridge mode the container has a private network namespace: both `localhost` and `127.0.0.1` resolve to the *container's own* loopback, not the host. If `S3_ENDPOINT` is configured as an internal Docker-compose service name (e.g. a private MinIO instance on a compose network) that name will not be resolvable from the default bridge network, causing every artifact upload to fail silently in production. The `'localhost' → '127.0.0.1'` substitution at line 583 does not address this gap — it predates bridge mode and only made sense under host networking. How can I resolve this? If you propose a fix, please make it concise.
-
queueConsumer/src/actions/services/docker-daemon.service.ts, line 593-606 (link)The
environment.DEV ? 'host' : NetworkModeconditional is inlined and duplicated at both container-creation sites. If a third container creation method is added it is easy to miss this logic, silently applying the wrong network mode. Extracting it to a single named constant keeps the rule in one place.Prompt To Fix With AI
This is a comment left during a code review. Path: queueConsumer/src/actions/services/docker-daemon.service.ts Line: 593-606 Comment: The `environment.DEV ? 'host' : NetworkMode` conditional is inlined and duplicated at both container-creation sites. If a third container creation method is added it is easy to miss this logic, silently applying the wrong network mode. Extracting it to a single named constant keeps the rule in one place. How can I resolve this? If you propose a fix, please make it concise.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
queueConsumer/src/actions/services/docker-daemon.service.ts:590-597
**Artifact uploader bridge network may sever S3 connectivity**
The artifact uploader is a trusted internal container that must reach `S3_ENDPOINT` to persist action results. Before this PR it ran with host networking in all environments; now it uses bridge mode in production. In bridge mode the container has a private network namespace: both `localhost` and `127.0.0.1` resolve to the *container's own* loopback, not the host. If `S3_ENDPOINT` is configured as an internal Docker-compose service name (e.g. a private MinIO instance on a compose network) that name will not be resolvable from the default bridge network, causing every artifact upload to fail silently in production. The `'localhost' → '127.0.0.1'` substitution at line 583 does not address this gap — it predates bridge mode and only made sense under host networking.
### Issue 2 of 2
queueConsumer/src/actions/services/docker-daemon.service.ts:593-606
The `environment.DEV ? 'host' : NetworkMode` conditional is inlined and duplicated at both container-creation sites. If a third container creation method is added it is easy to miss this logic, silently applying the wrong network mode. Extracting it to a single named constant keeps the rule in one place.
```suggestion
NetworkMode: environment.DEV ? 'host' : NetworkMode, // keep in sync with startContainer
LogConfig,
CapDrop,
SecurityOpt,
PidsLimit,
Mounts: [
{
Target: '/out',
Source: volumeName,
Type: 'volume',
},
],
},
};
```
Reviews (1): Last reviewed commit: "Implement best practices for running unt..." | Re-trigger Greptile
This PR closes #122 by implementing container-level hardening settings (dropped capabilities, no privilege escalation, and bridge network mode isolation in production) for executing untrusted workloads.