Skip to content

Envd certs support#2264

Draft
sitole wants to merge 9 commits intomainfrom
feat/envd-ca-certificates
Draft

Envd certs support#2264
sitole wants to merge 9 commits intomainfrom
feat/envd-ca-certificates

Conversation

@sitole
Copy link
Copy Markdown
Member

@sitole sitole commented Mar 30, 2026

No description provided.

@cursor
Copy link
Copy Markdown

cursor bot commented Mar 30, 2026

PR Summary

Medium Risk
Touches sandbox initialization and executes OS-level trust-store updates (update-ca-certificates) based on remote input; while names are sanitized, bad PEM or missing tooling/permissions could break TLS or slow init.

Overview
Adds end-to-end support for injecting custom CA certificates into sandboxes: the orchestrator’s egress proxy can now expose CACertificates() which are carried on Sandbox and sent in the /init payload, and envd writes the provided PEM certs into the system CA directory (with filename validation) before running update-ca-certificates so sandbox TLS can trust the proxy/extra roots.

Written by Cursor Bugbot for commit 377807f. This will update automatically on new commits. Configure here.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: CA cert installation is async causing a race condition
    • SetData now schedules CA certificate installation in the same WaitGroup as volume mounts and waits before returning so /init only responds after cert setup completes.
  • ✅ Fixed: Missing compile-time interface check for NoopEgressProxy
    • Added var _ EgressProxy = (*NoopEgressProxy)(nil) to enforce compile-time interface conformance for NoopEgressProxy.

Create PR

Or push these changes by commenting:

@cursor push 130914f64c
Preview (130914f64c)
diff --git a/packages/envd/internal/api/init.go b/packages/envd/internal/api/init.go
--- a/packages/envd/internal/api/init.go
+++ b/packages/envd/internal/api/init.go
@@ -217,12 +217,15 @@
 		a.defaults.Workdir = data.DefaultWorkdir
 	}
 
+	var wg sync.WaitGroup
 	if data.CaCertificates != nil && len(*data.CaCertificates) > 0 {
-		go a.installCACerts(context.WithoutCancel(ctx), *data.CaCertificates)
+		certs := *data.CaCertificates
+		wg.Go(func() {
+			a.installCACerts(context.WithoutCancel(ctx), certs)
+		})
 	}
 
 	if data.VolumeMounts != nil {
-		var wg sync.WaitGroup
 		for _, volume := range *data.VolumeMounts {
 			logger.Debug().Msgf("Mounting %s at %q", volume.NfsTarget, volume.Path)
 
@@ -230,9 +233,8 @@
 				a.setupNfs(context.WithoutCancel(ctx), volume.NfsTarget, volume.Path)
 			})
 		}
-
-		wg.Wait()
 	}
+	wg.Wait()
 
 	return nil
 }

diff --git a/packages/orchestrator/pkg/sandbox/network/egressproxy.go b/packages/orchestrator/pkg/sandbox/network/egressproxy.go
--- a/packages/orchestrator/pkg/sandbox/network/egressproxy.go
+++ b/packages/orchestrator/pkg/sandbox/network/egressproxy.go
@@ -22,6 +22,8 @@
 // NoopEgressProxy is a no-op implementation of EgressProxy.
 type NoopEgressProxy struct{}
 
+var _ EgressProxy = (*NoopEgressProxy)(nil)
+
 func NewNoopEgressProxy() NoopEgressProxy {
 	return NoopEgressProxy{}
 }

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

@sitole sitole force-pushed the feat/envd-ca-certificates branch from 217fb35 to f181ed7 Compare March 30, 2026 12:11
sitole added 3 commits March 30, 2026 14:42
CA certs needs to be apply in sync to fix case where init will finish
before CA store update and some commands can fails as cert used by proxy
is still not trusted.

Running install CA function in sync mode makes init slower.
This change introduces optimization for case where both volumes (also
slowing init request) and CA are upsed, function will use shared wait
groop and wait only once.
Previously a WriteFile failure for any cert caused an immediate return,
leaving already-written certs on disk but never registered in the system
trust store via update-ca-certificates. This meant a single bad cert
could silently break TLS for all other injected certs.

Changed the early return to continue so every cert is attempted and
update-ca-certificates always runs at the end regardless of individual
write failures. Errors are still logged per cert.

Added a test that forces a write failure by pre-creating the target path
as a directory, then asserts the next cert in the list is still written.
CodeQL flagged c.Name as user-controlled input flowing into a path
expression (go/path-injection). Although the cert name is set by the
orchestrator's egress proxy rather than a direct user, it is
deserialized from the HTTP request body so CodeQL's taint analysis
correctly traces it as external input.

Fix: validate the name against a strict allowlist regexp
(^[a-zA-Z0-9_-]+$) before constructing the path, and reject any name
that contains dots, slashes, spaces, or other special characters.
This makes the intent explicit and removes the taint from CodeQL's
perspective without relying on filepath.Base as a sanitiser.

The existing path-traversal test is updated to assert the cert is now
rejected outright rather than silently sanitised. A second test covers
other invalid name forms (dots, slashes, empty string).
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

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