Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions sandbox-api/docs/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,38 @@ const docTemplate = `{
}
}
},
"/environment/reload": {
"post": {
"description": "Re-reads /bl/metadata and applies its environment to the sandbox-api process, so this process and every process started afterwards see the current values. Called by the guest init after an environment update; safe to call manually.",
"produces": [
"application/json"
],
"tags": [
"system"
],
"summary": "Reload environment from guest metadata",
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/handler.ReloadResponse"
}
},
"404": {
"description": "Not Found",
"schema": {
"$ref": "#/definitions/ErrorResponse"
}
},
"500": {
"description": "Internal Server Error",
"schema": {
"$ref": "#/definitions/ErrorResponse"
}
}
}
}
},
"/filesystem-content-search/{path}": {
"get": {
"description": "Searches for text content inside files using ripgrep. Returns matching lines with context.",
Expand Down Expand Up @@ -2758,6 +2790,20 @@ const docTemplate = `{
}
}
},
"handler.ReloadResponse": {
"type": "object",
"properties": {
"applied": {
"type": "integer"
},
"generation": {
"type": "integer"
},
"removed": {
"type": "integer"
}
}
},
"process.UpgradeState": {
"type": "string",
"enum": [
Expand Down
34 changes: 34 additions & 0 deletions sandbox-api/docs/openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,31 @@ paths:
summary: Detach a drive from a local path
tags:
- drive
/environment/reload:
post:
description: Re-reads /bl/metadata and applies its environment to the sandbox-api process, so this process and every process started afterwards see the current values. Called by the guest init after an environment update; safe to call manually.
responses:
"200":
description: OK
content:
application/json:
schema:
$ref: "#/components/schemas/handler.ReloadResponse"
"404":
description: Not Found
content:
application/json:
schema:
$ref: "#/components/schemas/ErrorResponse"
"500":
description: Internal Server Error
content:
application/json:
schema:
$ref: "#/components/schemas/ErrorResponse"
summary: Reload environment from guest metadata
tags:
- system
"/filesystem-content-search/{path}":
get:
description: Searches for text content inside files using ripgrep. Returns matching lines with context.
Expand Down Expand Up @@ -2181,6 +2206,15 @@ components:
uploadedAt:
type: string
type: object
handler.ReloadResponse:
properties:
applied:
type: integer
generation:
type: integer
removed:
type: integer
type: object
process.UpgradeState:
enum:
- idle
Expand Down
5 changes: 5 additions & 0 deletions sandbox-api/src/api/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,11 @@ func SetupRouter(disableRequestLogging bool, enableProcessingTime bool) *gin.Eng
logrus.Info("Terminal endpoint disabled via DISABLE_TERMINAL environment variable")
}

// Environment routes: the guest init calls this after applying a new
// metadata generation so the process env follows without a restart.
environmentHandler := handler.NewEnvironmentHandler()
r.POST("/environment/reload", environmentHandler.HandleReload)

// System routes
r.POST("/upgrade", systemHandler.HandleUpgrade)
r.HEAD("/upgrade", head)
Expand Down
124 changes: 124 additions & 0 deletions sandbox-api/src/handler/environment.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
package handler

import (
"net/http"
"os"
"sync"

"github.com/gin-gonic/gin"
"github.com/sirupsen/logrus"
)

// defaultMetadataPath is the guest metadata document served by the host
// (vmm-manager) through the initrd's FUSE mount. It always reflects the
// current generation: on an environment update the host persists the new set,
// the initrd refetches the document, then pings POST /environment/reload so
// this process adopts it. Absent on VMs booted from an initrd that predates
// the metadata protocol. Overridable through BL_METADATA_PATH.
const defaultMetadataPath = "/bl/metadata"

func metadataPath() string {
if path := os.Getenv("BL_METADATA_PATH"); path != "" {
return path
}
return defaultMetadataPath
}
Comment on lines +20 to +25

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🔍 Metadata can override the very variable that selects the metadata path

metadataPath() reads BL_METADATA_PATH from the process environment, and HandleReload then applies arbitrary key/values from the document via os.Setenv. If a metadata document ever carries BL_METADATA_PATH, subsequent reloads read from a different file (and if a later generation drops the key, the path silently reverts). Worth considering skipping this key when applying, or resolving the path once at handler construction.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6b36cc0 — the metadata path is now resolved once at handler construction, so a document carrying BL_METADATA_PATH can no longer redirect subsequent reloads.


// metadataDocument is the subset of the guest metadata document this handler
// reads. The environment carried is the host's complete set, so a variable the
// host no longer has must be unset here too.
type metadataDocument struct {
Generation int64 `json:"generation"`
Environment map[string]string `json:"environment"`
}

// EnvironmentHandler applies environment updates to the sandbox-api process
// itself. A live process's environment cannot be changed from outside, so the
// initrd notifies this endpoint after applying a new metadata generation; the
// values are set with os.Setenv, which also makes every process spawned
// afterwards (process API, terminals, restarts) inherit them.
type EnvironmentHandler struct {
*BaseHandler

mu sync.Mutex
// Keys applied from the metadata document, so a key the next generation no
// longer carries is unset rather than left behind. Keys that were never in
// the document (the image ENV, boot-time variables) are never touched.
applied map[string]struct{}
}

// NewEnvironmentHandler creates a new environment handler.
func NewEnvironmentHandler() *EnvironmentHandler {
return &EnvironmentHandler{
BaseHandler: NewBaseHandler(),
applied: map[string]struct{}{},
}
}

// ReloadResponse is the response body for the environment reload endpoint.
type ReloadResponse struct {
Generation int64 `json:"generation"`
Applied int `json:"applied"`
Removed int `json:"removed"`
}

// HandleReload reloads the environment from the guest metadata document
// @Summary Reload environment from guest metadata
// @Description Re-reads /bl/metadata and applies its environment to the sandbox-api process, so this process and every process started afterwards see the current values. Called by the guest init after an environment update; safe to call manually.
// @Tags system
// @Produce json
// @Success 200 {object} ReloadResponse
// @Failure 404 {object} ErrorResponse
// @Failure 500 {object} ErrorResponse
// @Router /environment/reload [post]
func (h *EnvironmentHandler) HandleReload(c *gin.Context) {
raw, err := os.ReadFile(metadataPath())
if err != nil {
if os.IsNotExist(err) {
h.SendError(c, http.StatusNotFound, err)
return
}
h.SendError(c, http.StatusInternalServerError, err)
return
}

var doc metadataDocument
if err := json.Unmarshal(raw, &doc); err != nil {
h.SendError(c, http.StatusInternalServerError, err)
return
}

h.mu.Lock()
defer h.mu.Unlock()

removed := 0
for key := range h.applied {
if _, ok := doc.Environment[key]; !ok {
if err := os.Unsetenv(key); err == nil {
removed++
}
delete(h.applied, key)
}
}
applied := 0
for key, value := range doc.Environment {
if err := os.Setenv(key, value); err != nil {
logrus.WithError(err).WithField("key", key).Warn("Failed to set environment variable")
continue
}
h.applied[key] = struct{}{}
applied++
Comment thread
cursor[bot] marked this conversation as resolved.
}
Comment on lines +101 to +118

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🟡 Variables that also exist in the image configuration are erased instead of restored when dropped from metadata

A variable that the container image already defined is deleted outright (os.Unsetenv(key) at sandbox-api/src/handler/environment.go:97) once it disappears from the metadata, instead of falling back to the value the image originally provided, so settings silently vanish.
Impact: If an environment update temporarily overrides a built-in setting (for example a search path or a service URL) and later drops it, the sandbox and every program it launches afterwards run with that setting completely missing rather than with the original default.

No snapshot of pre-existing values before overwriting

HandleReload tracks only the set of keys it applied (h.applied), not their prior values. First reload: metadata contains KEY=override, so os.Setenv("KEY", "override") replaces the image-provided value and h.applied["KEY"] is recorded (sandbox-api/src/handler/environment.go:104-111). Next reload where metadata no longer carries KEY: the loop at sandbox-api/src/handler/environment.go:95-102 unsets it, so the original image value is lost for this process and, via buildProcessEnv starting from os.Environ() (sandbox-api/src/handler/process/process.go:141), for every process spawned afterwards. Recording the previous value (and whether it existed) when first overriding would allow restoring instead of unsetting.

Prompt for agents
In sandbox-api/src/handler/environment.go, EnvironmentHandler only remembers which keys came from the metadata document (h.applied as a set), not what the process environment held before those keys were first overridden. When a later metadata generation drops a key, the handler unsets it, which erases any value that originally came from the container image ENV or boot-time setup rather than restoring it. Consider changing h.applied to a map from key to the previous value plus a flag indicating whether the key existed before, captured only the first time the handler overrides that key; on removal, restore the previous value if it existed, otherwise unset.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6b36cc0applied now records the value (and existence) the process held before the first override; when a later generation drops a key, that original value is restored instead of the variable being erased. Covered by TestHandleReloadRestoresPreExistingValue.


logrus.WithFields(logrus.Fields{
"generation": doc.Generation,
"applied": applied,
"removed": removed,
}).Info("Environment reloaded from guest metadata")

c.JSON(http.StatusOK, ReloadResponse{
Generation: doc.Generation,
Applied: applied,
Removed: removed,
})
}
65 changes: 65 additions & 0 deletions sandbox-api/src/handler/environment_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package handler

import (
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"testing"

"github.com/gin-gonic/gin"
)

func performReload(t *testing.T, h *EnvironmentHandler) *httptest.ResponseRecorder {
t.Helper()
gin.SetMode(gin.TestMode)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Request = httptest.NewRequest(http.MethodPost, "/environment/reload", nil)
h.HandleReload(c)
return w
}

func TestHandleReloadAppliesAndRemoves(t *testing.T) {
doc := filepath.Join(t.TempDir(), "metadata")
t.Setenv("BL_METADATA_PATH", doc)

h := NewEnvironmentHandler()
t.Cleanup(func() {
os.Unsetenv("RELOAD_TEST_A")
os.Unsetenv("RELOAD_TEST_B")
})

if err := os.WriteFile(doc, []byte(`{"generation":1,"environment":{"RELOAD_TEST_A":"1","RELOAD_TEST_B":"2"}}`), 0o600); err != nil {
t.Fatal(err)
}
if w := performReload(t, h); w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
if os.Getenv("RELOAD_TEST_A") != "1" || os.Getenv("RELOAD_TEST_B") != "2" {
t.Fatalf("environment not applied")
}

// A variable the next generation no longer carries is unset; one it still
// carries is updated.
if err := os.WriteFile(doc, []byte(`{"generation":2,"environment":{"RELOAD_TEST_A":"updated"}}`), 0o600); err != nil {
t.Fatal(err)
}
if w := performReload(t, h); w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
if os.Getenv("RELOAD_TEST_A") != "updated" {
t.Fatalf("environment not updated")
}
if _, ok := os.LookupEnv("RELOAD_TEST_B"); ok {
t.Fatalf("removed variable still set")
}
}

func TestHandleReloadWithoutMetadata(t *testing.T) {
t.Setenv("BL_METADATA_PATH", filepath.Join(t.TempDir(), "missing"))

if w := performReload(t, NewEnvironmentHandler()); w.Code != http.StatusNotFound {
t.Fatalf("expected 404, got %d", w.Code)
}
}
Loading