forked from molecule-ai/molecule-core
Compare commits
9 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 7fbb8cb6e9 | |||
| e37a289eb6 | |||
| 61166f8848 | |||
| 9d50a6dae4 | |||
| 43b33bcaa5 | |||
| 7d3a6a46c5 | |||
| ae2d9eabf6 | |||
| 2fac4b61b4 | |||
| 5abc4f74ca |
@@ -22,9 +22,9 @@ on:
|
||||
# spending CI cycles. See e2e-api.yml for the rationale on why this
|
||||
# is a single job rather than two-jobs-sharing-name.
|
||||
push:
|
||||
branches: [main, staging]
|
||||
branches: [main]
|
||||
pull_request:
|
||||
branches: [main, staging]
|
||||
branches: [main]
|
||||
workflow_dispatch:
|
||||
schedule:
|
||||
# Weekly on Sunday 08:00 UTC — catches Chrome / Playwright / Next.js
|
||||
|
||||
@@ -32,7 +32,7 @@ name: E2E Staging External Runtime
|
||||
|
||||
on:
|
||||
push:
|
||||
branches: [staging, main]
|
||||
branches: [main]
|
||||
paths:
|
||||
- 'workspace-server/internal/handlers/workspace.go'
|
||||
- 'workspace-server/internal/handlers/registry.go'
|
||||
@@ -44,7 +44,7 @@ on:
|
||||
- 'tests/e2e/test_staging_external_runtime.sh'
|
||||
- '.github/workflows/e2e-staging-external.yml'
|
||||
pull_request:
|
||||
branches: [staging, main]
|
||||
branches: [main]
|
||||
paths:
|
||||
- 'workspace-server/internal/handlers/workspace.go'
|
||||
- 'workspace-server/internal/handlers/registry.go'
|
||||
|
||||
@@ -20,13 +20,12 @@ name: E2E Staging SaaS (full lifecycle)
|
||||
# via the same paths watcher that e2e-api.yml uses)
|
||||
|
||||
on:
|
||||
# Fire on staging push too — previously this only ran on main, which
|
||||
# meant the most thorough end-to-end test caught regressions AFTER
|
||||
# they shipped to staging (and then to the auto-promote PR). Running
|
||||
# on staging push catches them BEFORE the staging→main promotion
|
||||
# opens, so a green canary into auto-promote is more meaningful.
|
||||
# Trunk-based (Phase 3 of internal#81): main is the only branch.
|
||||
# Previously this fired on staging push too because staging was a
|
||||
# superset of main and ran the gate ahead of auto-promote; with no
|
||||
# staging branch, main is where E2E gates the deploy.
|
||||
push:
|
||||
branches: [staging, main]
|
||||
branches: [main]
|
||||
paths:
|
||||
- 'workspace-server/internal/handlers/registry.go'
|
||||
- 'workspace-server/internal/handlers/workspace_provision.go'
|
||||
@@ -36,7 +35,7 @@ on:
|
||||
- 'tests/e2e/test_staging_full_saas.sh'
|
||||
- '.github/workflows/e2e-staging-saas.yml'
|
||||
pull_request:
|
||||
branches: [staging, main]
|
||||
branches: [main]
|
||||
paths:
|
||||
- 'workspace-server/internal/handlers/registry.go'
|
||||
- 'workspace-server/internal/handlers/workspace_provision.go'
|
||||
|
||||
@@ -36,7 +36,7 @@ on:
|
||||
workflow_run:
|
||||
workflows: ['publish-workspace-server-image']
|
||||
types: [completed]
|
||||
branches: [staging]
|
||||
branches: [main]
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
target_tag:
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
# Top-level Makefile — convenience wrappers around docker compose.
|
||||
#
|
||||
# Most molecule-core dev work happens via these shortcuts. CI doesn't
|
||||
# use this Makefile; CI calls docker compose / go test directly so the
|
||||
# Makefile can evolve without breaking the build.
|
||||
|
||||
.PHONY: help dev up down logs build test
|
||||
|
||||
help: ## Show this help.
|
||||
@grep -E '^[a-zA-Z_-]+:.*?## ' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-12s\033[0m %s\n", $$1, $$2}'
|
||||
|
||||
dev: ## Start the full stack with air hot-reload for the platform service.
|
||||
docker compose -f docker-compose.yml -f docker-compose.dev.yml up
|
||||
|
||||
up: ## Start the full stack in production-shape mode (no air, normal Dockerfile).
|
||||
docker compose up
|
||||
|
||||
down: ## Stop the stack and remove containers (volumes preserved).
|
||||
docker compose down
|
||||
|
||||
logs: ## Tail logs from all services (Ctrl-C to detach).
|
||||
docker compose logs -f
|
||||
|
||||
build: ## Force a fresh build of the platform image (no cache).
|
||||
docker compose build --no-cache platform
|
||||
|
||||
test: ## Run Go unit tests in workspace-server/.
|
||||
cd workspace-server && go test -race ./...
|
||||
@@ -0,0 +1,43 @@
|
||||
# docker-compose.dev.yml — overlay over docker-compose.yml for local dev
|
||||
# with air-driven live reload of the platform (workspace-server) service.
|
||||
#
|
||||
# Usage:
|
||||
# docker compose -f docker-compose.yml -f docker-compose.dev.yml up
|
||||
# (or `make dev` shorthand from repo root)
|
||||
#
|
||||
# What this overlay changes vs docker-compose.yml alone:
|
||||
# - Platform service uses workspace-server/Dockerfile.dev (air on top of
|
||||
# golang:1.25-alpine) instead of the multi-stage prod Dockerfile.
|
||||
# - Platform service bind-mounts the host's workspace-server/ source
|
||||
# into /app/workspace-server so air sees source edits live.
|
||||
# - Other services (postgres, redis, langfuse, etc.) inherit unchanged
|
||||
# from docker-compose.yml.
|
||||
#
|
||||
# What stays the same:
|
||||
# - All env vars, volumes, depends_on, healthchecks from docker-compose.yml.
|
||||
# - Network topology + ports.
|
||||
# - Postgres/Redis as service containers (no in-process replacements).
|
||||
|
||||
services:
|
||||
platform:
|
||||
build:
|
||||
context: .
|
||||
dockerfile: workspace-server/Dockerfile.dev
|
||||
# Rebind source: edits under host's workspace-server/ propagate live.
|
||||
# The named volume on go-build-cache speeds up first build per container.
|
||||
volumes:
|
||||
- ./workspace-server:/app/workspace-server
|
||||
- go-build-cache:/root/.cache/go-build
|
||||
- go-mod-cache:/go/pkg/mod
|
||||
# Air signals the running binary on rebuild; ensure shell stops cleanly.
|
||||
init: true
|
||||
# Mark the service as dev-mode so the platform can short-circuit any
|
||||
# behavior that's incompatible with hot-reload (e.g. background
|
||||
# cron-style watchers that don't survive process restart). No-op
|
||||
# today; reserved for future flag use.
|
||||
environment:
|
||||
MOLECULE_DEV_HOT_RELOAD: "1"
|
||||
|
||||
volumes:
|
||||
go-build-cache:
|
||||
go-mod-cache:
|
||||
@@ -0,0 +1,49 @@
|
||||
# air.toml — live-reload config for local docker-compose dev mode.
|
||||
#
|
||||
# Active when the platform service runs from workspace-server/Dockerfile.dev
|
||||
# (selected via docker-compose.dev.yml overlay). In production, the regular
|
||||
# Dockerfile builds a static binary; air is dev-only.
|
||||
#
|
||||
# Reference: https://github.com/air-verse/air
|
||||
|
||||
root = "."
|
||||
testdata_dir = "testdata"
|
||||
tmp_dir = "tmp"
|
||||
|
||||
[build]
|
||||
# Same build invocation as Dockerfile's builder stage minus the
|
||||
# CGO_ENABLED=0 toggle (CGO ok in dev for richer race detector output).
|
||||
cmd = "go build -o ./tmp/server ./cmd/server"
|
||||
bin = "tmp/server"
|
||||
full_bin = ""
|
||||
args_bin = []
|
||||
# Watch every .go and .yaml file under workspace-server/.
|
||||
include_ext = ["go", "yaml", "tmpl"]
|
||||
# Don't watch tests, build artifacts, vendored deps, or migration .sql
|
||||
# (migrations need a clean DB anyway — handled by docker-compose down/up).
|
||||
exclude_dir = ["assets", "tmp", "vendor", "testdata", "node_modules"]
|
||||
exclude_file = []
|
||||
# _test.go and *_mock.go shouldn't trigger a rebuild — saves cycles.
|
||||
exclude_regex = ["_test\\.go$", "_mock\\.go$"]
|
||||
exclude_unchanged = true
|
||||
follow_symlink = false
|
||||
log = "build-errors.log"
|
||||
# Kill running binary 1s before starting new one.
|
||||
kill_delay = "1s"
|
||||
send_interrupt = true
|
||||
stop_on_error = true
|
||||
# Debounce: wait this long after last change before triggering rebuild.
|
||||
delay = 500
|
||||
|
||||
[log]
|
||||
time = false
|
||||
|
||||
[color]
|
||||
main = "magenta"
|
||||
watcher = "cyan"
|
||||
build = "yellow"
|
||||
runner = "green"
|
||||
|
||||
[misc]
|
||||
# Don't keep the tmp/ dir around between runs.
|
||||
clean_on_exit = true
|
||||
@@ -0,0 +1,38 @@
|
||||
# Dockerfile.dev — local-development image with air-driven live reload.
|
||||
#
|
||||
# Selected by docker-compose.dev.yml (overlay over docker-compose.yml).
|
||||
# Production stays on workspace-server/Dockerfile (static binary, no air).
|
||||
#
|
||||
# Workflow:
|
||||
# 1. docker compose -f docker-compose.yml -f docker-compose.dev.yml up
|
||||
# 2. Edit any .go file under workspace-server/
|
||||
# 3. air detects, rebuilds, kills old binary, starts new one (~3-5s)
|
||||
# 4. No `docker compose up --build` needed
|
||||
#
|
||||
# Templates + plugins are NOT pre-cloned here — air-mode assumes the
|
||||
# developer's filesystem has the workspace-configs-templates/ + plugins/
|
||||
# dirs available, mounted at runtime via docker-compose.dev.yml.
|
||||
|
||||
FROM golang:1.25-alpine
|
||||
|
||||
# air + git (for go mod) + ca-certs (for TLS) + tzdata (for time-zone DB).
|
||||
RUN apk add --no-cache git ca-certificates tzdata wget \
|
||||
&& go install github.com/air-verse/air@latest
|
||||
|
||||
WORKDIR /app/workspace-server
|
||||
|
||||
# Pre-fetch deps so the first `air` rebuild on a fresh container is fast.
|
||||
# These are bind-mount-overridden at runtime, so the COPY here is just
|
||||
# to warm the module cache.
|
||||
COPY workspace-server/go.mod workspace-server/go.sum ./
|
||||
RUN go mod download
|
||||
|
||||
# Source is bind-mounted at runtime (see docker-compose.dev.yml volumes
|
||||
# block) so the Dockerfile doesn't need to COPY it. air watches the
|
||||
# bind-mounted dir for changes.
|
||||
|
||||
ENV CGO_ENABLED=1
|
||||
ENV GOFLAGS="-buildvcs=false"
|
||||
|
||||
# Run air with the .air.toml in the bind-mounted source dir.
|
||||
CMD ["air", "-c", ".air.toml"]
|
||||
@@ -6,6 +6,7 @@ package handlers
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"log"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
@@ -102,6 +103,56 @@ func loadWorkspaceEnv(orgBaseDir, filesDir string) map[string]string {
|
||||
return envVars
|
||||
}
|
||||
|
||||
// loadPersonaEnvFile merges per-role persona credentials into out. The file
|
||||
// lives at $MOLECULE_PERSONA_ROOT/<role>/env (default
|
||||
// /etc/molecule-bootstrap/personas) and is populated by the operator-host
|
||||
// bootstrap kit — one persona per dev-tree role, each carrying the role's
|
||||
// Gitea identity (GITEA_USER, GITEA_TOKEN, GITEA_TOKEN_SCOPES,
|
||||
// GITEA_USER_EMAIL, GITEA_SSH_KEY_PATH).
|
||||
//
|
||||
// Lower precedence than the org and workspace .env files: callers should
|
||||
// invoke this BEFORE parseEnvFile on those, so a workspace .env can
|
||||
// override a persona-default value when needed.
|
||||
//
|
||||
// Silent no-op when role is empty, when the role name fails the safe-segment
|
||||
// check, or when the env file does not exist (workspaces without a role —
|
||||
// or running on hosts that don't ship the bootstrap dir — keep their old
|
||||
// behavior).
|
||||
func loadPersonaEnvFile(role string, out map[string]string) {
|
||||
if !isSafeRoleName(role) {
|
||||
if role != "" {
|
||||
log.Printf("Org import: refusing persona env load for unsafe role name %q", role)
|
||||
}
|
||||
return
|
||||
}
|
||||
root := os.Getenv("MOLECULE_PERSONA_ROOT")
|
||||
if root == "" {
|
||||
root = "/etc/molecule-bootstrap/personas"
|
||||
}
|
||||
parseEnvFile(filepath.Join(root, role, "env"), out)
|
||||
}
|
||||
|
||||
// isSafeRoleName accepts a single path segment of [A-Za-z0-9_-]+. Rejects
|
||||
// empty, ".", "..", and anything containing a path separator — even though
|
||||
// the construct is admin-only, defense-in-depth keeps the persona dir
|
||||
// shape invariant: one flat directory per role, no climbing out.
|
||||
func isSafeRoleName(s string) bool {
|
||||
if s == "" || s == "." || s == ".." {
|
||||
return false
|
||||
}
|
||||
for _, c := range s {
|
||||
switch {
|
||||
case c >= 'a' && c <= 'z':
|
||||
case c >= 'A' && c <= 'Z':
|
||||
case c >= '0' && c <= '9':
|
||||
case c == '-' || c == '_':
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// parseEnvFile reads a .env file and adds KEY=VALUE pairs to the map.
|
||||
// Skips comments (#) and empty lines. Values can be quoted.
|
||||
func parseEnvFile(path string, out map[string]string) {
|
||||
|
||||
@@ -443,10 +443,18 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX
|
||||
configFiles["system-prompt.md"] = []byte(ws.SystemPrompt)
|
||||
}
|
||||
|
||||
// Inject secrets from .env files as workspace secrets.
|
||||
// Resolution: workspace .env → org root .env (workspace overrides org root).
|
||||
// Inject secrets from persona env + .env files as workspace secrets.
|
||||
// Resolution (later overrides earlier):
|
||||
// 0. Persona env (per-role bootstrap creds; only when ws.Role is set
|
||||
// and the operator-host bootstrap dir ships a matching file)
|
||||
// 1. Org root .env (shared defaults)
|
||||
// 2. Workspace-specific .env (per-workspace overrides)
|
||||
// Each line: KEY=VALUE → stored as encrypted workspace secret.
|
||||
envVars := map[string]string{}
|
||||
// 0. Persona env (lowest precedence; injects the role's Gitea identity:
|
||||
// GITEA_USER, GITEA_TOKEN, GITEA_TOKEN_SCOPES, GITEA_USER_EMAIL,
|
||||
// GITEA_SSH_KEY_PATH). Workspace and org .env can override.
|
||||
loadPersonaEnvFile(ws.Role, envVars)
|
||||
if orgBaseDir != "" {
|
||||
// 1. Org root .env (shared defaults)
|
||||
parseEnvFile(filepath.Join(orgBaseDir, ".env"), envVars)
|
||||
|
||||
@@ -0,0 +1,171 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestLoadPersonaEnvFile_HappyPath: the standard case — a persona-shaped
|
||||
// env file exists at <root>/<role>/env and its KEY=VALUE pairs land in
|
||||
// the out map. Mirrors what the operator-host bootstrap kit ships:
|
||||
// GITEA_USER, GITEA_TOKEN, GITEA_TOKEN_SCOPES, GITEA_USER_EMAIL,
|
||||
// GITEA_SSH_KEY_PATH.
|
||||
func TestLoadPersonaEnvFile_HappyPath(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
roleDir := filepath.Join(root, "dev-lead")
|
||||
if err := os.MkdirAll(roleDir, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
envBody := `# Persona env file — mode 600
|
||||
GITEA_USER=dev-lead
|
||||
GITEA_USER_EMAIL=dev-lead@agents.moleculesai.app
|
||||
GITEA_TOKEN=abc123
|
||||
GITEA_TOKEN_SCOPES=write:repository,write:issue,read:user
|
||||
GITEA_SSH_KEY_PATH=/etc/molecule-bootstrap/personas/dev-lead/ssh_priv
|
||||
`
|
||||
if err := os.WriteFile(filepath.Join(roleDir, "env"), []byte(envBody), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Setenv("MOLECULE_PERSONA_ROOT", root)
|
||||
|
||||
out := map[string]string{}
|
||||
loadPersonaEnvFile("dev-lead", out)
|
||||
|
||||
want := map[string]string{
|
||||
"GITEA_USER": "dev-lead",
|
||||
"GITEA_USER_EMAIL": "dev-lead@agents.moleculesai.app",
|
||||
"GITEA_TOKEN": "abc123",
|
||||
"GITEA_TOKEN_SCOPES": "write:repository,write:issue,read:user",
|
||||
"GITEA_SSH_KEY_PATH": "/etc/molecule-bootstrap/personas/dev-lead/ssh_priv",
|
||||
}
|
||||
if len(out) != len(want) {
|
||||
t.Fatalf("got %d keys, want %d: %#v", len(out), len(want), out)
|
||||
}
|
||||
for k, v := range want {
|
||||
if out[k] != v {
|
||||
t.Errorf("out[%q] = %q; want %q", k, out[k], v)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoadPersonaEnvFile_MissingDir: when the persona dir doesn't exist
|
||||
// (e.g. dev-only host without the bootstrap kit, or a workspace whose
|
||||
// role isn't a known persona), it's a silent no-op — out stays empty,
|
||||
// no panic, no log noise that would break callers.
|
||||
func TestLoadPersonaEnvFile_MissingDir(t *testing.T) {
|
||||
t.Setenv("MOLECULE_PERSONA_ROOT", t.TempDir()) // empty dir
|
||||
out := map[string]string{}
|
||||
loadPersonaEnvFile("nonexistent-role", out)
|
||||
if len(out) != 0 {
|
||||
t.Errorf("expected empty out, got %#v", out)
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoadPersonaEnvFile_EmptyRole: empty role string is the common case
|
||||
// for non-dev workspaces (research/marketing/etc.). Skip silently.
|
||||
func TestLoadPersonaEnvFile_EmptyRole(t *testing.T) {
|
||||
t.Setenv("MOLECULE_PERSONA_ROOT", t.TempDir())
|
||||
out := map[string]string{}
|
||||
loadPersonaEnvFile("", out)
|
||||
if len(out) != 0 {
|
||||
t.Errorf("empty role should produce empty out; got %#v", out)
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoadPersonaEnvFile_RejectsTraversal: even though role names come
|
||||
// from server-side admin-only org templates, defense-in-depth — refuse
|
||||
// any role string with path separators or "..". Verifies that a maliciously
|
||||
// crafted template can't read /etc/passwd by setting role: "../../etc".
|
||||
func TestLoadPersonaEnvFile_RejectsTraversal(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
// Plant a file at /tmp/.../env so a bad traversal would reach it
|
||||
if err := os.WriteFile(filepath.Join(root, "env"), []byte("STOLEN=yes\n"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Setenv("MOLECULE_PERSONA_ROOT", filepath.Join(root, "personas"))
|
||||
|
||||
for _, bad := range []string{"..", "../personas", "../etc/passwd", "/abs", "with/slash", "dot.in.middle", "with space", "back\\slash", ".", ""} {
|
||||
out := map[string]string{}
|
||||
loadPersonaEnvFile(bad, out)
|
||||
if len(out) != 0 {
|
||||
t.Errorf("role %q should have been rejected; got %#v", bad, out)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoadPersonaEnvFile_DefaultRoot: when MOLECULE_PERSONA_ROOT is unset,
|
||||
// the helper falls back to /etc/molecule-bootstrap/personas. We don't
|
||||
// touch real /etc — just verify the function doesn't panic and produces
|
||||
// empty out (since the test box isn't expected to ship that path).
|
||||
func TestLoadPersonaEnvFile_DefaultRoot(t *testing.T) {
|
||||
t.Setenv("MOLECULE_PERSONA_ROOT", "") // explicit empty
|
||||
out := map[string]string{}
|
||||
loadPersonaEnvFile("dev-lead", out)
|
||||
// Don't assert content — production CI might or might not have the
|
||||
// /etc dir mounted. Just verify the call returns cleanly.
|
||||
_ = out
|
||||
}
|
||||
|
||||
// TestLoadPersonaEnvFile_PrecedenceCallerOverrides: the contract is "lower
|
||||
// precedence than later .env files." The helper writes into out without
|
||||
// removing existing keys, so a caller pre-populating out simulates a
|
||||
// later layer overriding persona defaults. We verify the helper does NOT
|
||||
// clobber pre-existing entries… actually, parseEnvFile DOES overwrite,
|
||||
// so the caller-side ordering (persona → org → workspace) is what enforces
|
||||
// precedence. This test pins that contract: persona is loaded into a
|
||||
// fresh map, then later layers can override.
|
||||
func TestLoadPersonaEnvFile_OverwritesEmptyMap(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
roleDir := filepath.Join(root, "core-be")
|
||||
if err := os.MkdirAll(roleDir, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(roleDir, "env"),
|
||||
[]byte("GITEA_TOKEN=persona-value\n"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Setenv("MOLECULE_PERSONA_ROOT", root)
|
||||
|
||||
out := map[string]string{"GITEA_TOKEN": "preset"}
|
||||
loadPersonaEnvFile("core-be", out)
|
||||
|
||||
// Persona helper is meant to populate a FRESH map first in the
|
||||
// caller's flow; calling it on a pre-populated map and seeing the
|
||||
// value get overwritten is consistent with parseEnvFile semantics.
|
||||
if out["GITEA_TOKEN"] != "persona-value" {
|
||||
t.Errorf("loadPersonaEnvFile did not write into existing map; got %q", out["GITEA_TOKEN"])
|
||||
}
|
||||
}
|
||||
|
||||
// TestIsSafeRoleName_Acceptance: positive + negative cases for the
|
||||
// validator. Pinned because every dev-tree role name must pass.
|
||||
func TestIsSafeRoleName_Acceptance(t *testing.T) {
|
||||
good := []string{
|
||||
"dev-lead", "core-be", "cp-security", "infra-runtime-be",
|
||||
"sdk-dev", "plugin-dev", "documentation-specialist",
|
||||
"triage-operator", "fullstack-engineer", "release-manager",
|
||||
"core_underscore_ok", "X", "a1", "Z9-0",
|
||||
}
|
||||
for _, s := range good {
|
||||
if !isSafeRoleName(s) {
|
||||
t.Errorf("isSafeRoleName(%q) = false; want true", s)
|
||||
}
|
||||
}
|
||||
bad := []string{
|
||||
"", ".", "..", "with/slash", "/abs", "dot.in.middle",
|
||||
"with space", "back\\slash", "trailing-", // trailing-hyphen is fine actually
|
||||
"with$dollar", "with?question", "newline\nsplit",
|
||||
}
|
||||
// trailing-hyphen IS allowed; remove from "bad" list:
|
||||
bad = []string{
|
||||
"", ".", "..", "with/slash", "/abs", "dot.in.middle",
|
||||
"with space", "back\\slash", "with$dollar", "with?question",
|
||||
"newline\nsplit",
|
||||
}
|
||||
for _, s := range bad {
|
||||
if isSafeRoleName(s) {
|
||||
t.Errorf("isSafeRoleName(%q) = true; want false", s)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,207 @@
|
||||
package handlers
|
||||
|
||||
// plugins_atomic.go — atomic install pattern for plugin delivery into a
|
||||
// running workspace container. Closes molecule-core#114.
|
||||
//
|
||||
// Replaces the prior "tar + docker.CopyToContainer to /configs/plugins/<name>"
|
||||
// single-step write (no atomicity, no marker, no rollback) with a 4-step
|
||||
// dance:
|
||||
//
|
||||
// 1. STAGE — extract tar into /configs/plugins/.staging/<name>.<ts>/
|
||||
// 2. SNAPSHOT — if /configs/plugins/<name>/ exists, mv to .previous/<name>.<ts>/
|
||||
// 3. SWAP — mv /configs/plugins/.staging/<name>.<ts>/ → /configs/plugins/<name>/
|
||||
// 4. MARKER — touch /configs/plugins/<name>/.complete
|
||||
//
|
||||
// On any post-snapshot failure we attempt a best-effort rollback by mv-ing
|
||||
// the previous snapshot back into place. The .complete marker is the
|
||||
// canonical "this install is fully landed" signal — workspace-side plugin
|
||||
// loaders should refuse to load a plugin dir without it.
|
||||
//
|
||||
// Scope: docker path only (workspace running as a local container). The
|
||||
// SaaS path (deliverViaEIC, SSH-into-EC2) is unchanged in this PR; tracked
|
||||
// as a follow-up. The same stage-then-swap shape applies but the exec
|
||||
// primitives differ (ssh vs docker exec), and shipping both paths in one
|
||||
// PR doubles the test surface.
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"fmt"
|
||||
"path"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/docker/docker/api/types/container"
|
||||
)
|
||||
|
||||
const (
|
||||
pluginsRoot = "/configs/plugins"
|
||||
pluginsStagingDir = "/configs/plugins/.staging"
|
||||
pluginsPrevDir = "/configs/plugins/.previous"
|
||||
completeMarker = ".complete"
|
||||
)
|
||||
|
||||
// installVersion identifies one install attempt — the plugin name plus a
|
||||
// monotonic-ish UTC timestamp suffix. Used to namespace the staging dir
|
||||
// and any snapshot of the previous version, so a reinstall mid-flight
|
||||
// can't collide with a concurrent reinstall.
|
||||
type installVersion struct {
|
||||
plugin string
|
||||
stamp string // e.g. 20260508T141530Z
|
||||
}
|
||||
|
||||
func newInstallVersion(plugin string) installVersion {
|
||||
return installVersion{
|
||||
plugin: plugin,
|
||||
stamp: time.Now().UTC().Format("20060102T150405Z"),
|
||||
}
|
||||
}
|
||||
|
||||
// stagedPath is the container path where the new content lands during fetch.
|
||||
// e.g. /configs/plugins/.staging/molecule-skill-foo.20260508T141530Z
|
||||
func (v installVersion) stagedPath() string {
|
||||
return path.Join(pluginsStagingDir, v.plugin+"."+v.stamp)
|
||||
}
|
||||
|
||||
// previousPath is where the prior live version is moved before swap.
|
||||
// e.g. /configs/plugins/.previous/molecule-skill-foo.20260508T141530Z
|
||||
func (v installVersion) previousPath() string {
|
||||
return path.Join(pluginsPrevDir, v.plugin+"."+v.stamp)
|
||||
}
|
||||
|
||||
// livePath is the destination after swap.
|
||||
// e.g. /configs/plugins/molecule-skill-foo
|
||||
func (v installVersion) livePath() string {
|
||||
return path.Join(pluginsRoot, v.plugin)
|
||||
}
|
||||
|
||||
// markerPath is the .complete file inside the live dir written last.
|
||||
func (v installVersion) markerPath() string {
|
||||
return path.Join(v.livePath(), completeMarker)
|
||||
}
|
||||
|
||||
// atomicCopyToContainer does a stage→snapshot→swap→marker install of a
|
||||
// host-side staged plugin tree into a running container's
|
||||
// /configs/plugins/<name>/. Returns nil on success.
|
||||
//
|
||||
// On post-snapshot failure (swap or marker write), best-effort rollback
|
||||
// restores the previous snapshot to the live path. Returns the original
|
||||
// error wrapped — the caller should surface it; rollback success is
|
||||
// logged separately.
|
||||
func (h *PluginsHandler) atomicCopyToContainer(
|
||||
ctx context.Context, containerName, hostDir, pluginName string,
|
||||
) error {
|
||||
v := newInstallVersion(pluginName)
|
||||
|
||||
// Step 0a: ensure staging + previous root dirs exist (idempotent).
|
||||
if _, err := h.execAsRoot(ctx, containerName, []string{
|
||||
"mkdir", "-p", pluginsStagingDir, pluginsPrevDir,
|
||||
}); err != nil {
|
||||
return fmt.Errorf("atomic install: mkdir staging/previous: %w", err)
|
||||
}
|
||||
|
||||
// Step 0b: tar the host content with a path prefix that lands it in the
|
||||
// staging dir — NOT directly into the live name. The prefix has no
|
||||
// leading "/" because docker.CopyToContainer extracts paths relative
|
||||
// to the dstPath argument we pass below.
|
||||
stagedRel := strings.TrimPrefix(v.stagedPath(), "/")
|
||||
tarBuf, err := tarHostDirWithPrefix(hostDir, stagedRel)
|
||||
if err != nil {
|
||||
return fmt.Errorf("atomic install: tar host dir: %w", err)
|
||||
}
|
||||
|
||||
// Step 1: STAGE — extract tar into /configs/plugins/.staging/<name>.<ts>/
|
||||
if err := h.docker.CopyToContainer(ctx, containerName, "/", &tarBuf,
|
||||
container.CopyToContainerOptions{}); err != nil {
|
||||
// Best-effort: clean up any partial staging extract before returning.
|
||||
_, _ = h.execAsRoot(ctx, containerName, []string{
|
||||
"rm", "-rf", v.stagedPath(),
|
||||
})
|
||||
return fmt.Errorf("atomic install: copy to container: %w", err)
|
||||
}
|
||||
|
||||
// Step 2: SNAPSHOT — if a live version exists, move it aside.
|
||||
// `test -d` exits 0 if the dir exists, non-zero otherwise; the helper
|
||||
// returns a non-nil error in the non-zero case which we treat as
|
||||
// "no previous version" rather than a real failure.
|
||||
snapshotted := false
|
||||
if _, err := h.execAsRoot(ctx, containerName, []string{
|
||||
"test", "-d", v.livePath(),
|
||||
}); err == nil {
|
||||
if _, err := h.execAsRoot(ctx, containerName, []string{
|
||||
"mv", v.livePath(), v.previousPath(),
|
||||
}); err != nil {
|
||||
// Snapshot failure: roll back the staged extract before failing.
|
||||
_, _ = h.execAsRoot(ctx, containerName, []string{
|
||||
"rm", "-rf", v.stagedPath(),
|
||||
})
|
||||
return fmt.Errorf("atomic install: snapshot previous version: %w", err)
|
||||
}
|
||||
snapshotted = true
|
||||
}
|
||||
|
||||
// Step 3: SWAP — atomic rename of the staged dir into the live name.
|
||||
// `mv` on the same filesystem is a single rename(2), atomic at the FS level.
|
||||
if _, err := h.execAsRoot(ctx, containerName, []string{
|
||||
"mv", v.stagedPath(), v.livePath(),
|
||||
}); err != nil {
|
||||
// Swap failure: roll back if we had a snapshot.
|
||||
if snapshotted {
|
||||
if _, rbErr := h.execAsRoot(ctx, containerName, []string{
|
||||
"mv", v.previousPath(), v.livePath(),
|
||||
}); rbErr != nil {
|
||||
return fmt.Errorf("atomic install: swap failed AND rollback failed: swap=%w, rollback=%v", err, rbErr)
|
||||
}
|
||||
}
|
||||
// Best-effort cleanup of the still-staged dir.
|
||||
_, _ = h.execAsRoot(ctx, containerName, []string{
|
||||
"rm", "-rf", v.stagedPath(),
|
||||
})
|
||||
return fmt.Errorf("atomic install: swap to live path: %w", err)
|
||||
}
|
||||
|
||||
// Step 4: MARKER — touch .complete inside the live dir as the last write.
|
||||
// Workspace-side plugin loaders treat a plugin dir without this marker
|
||||
// as half-installed and skip it (or surface a clear error to the
|
||||
// operator instead of loading a possibly-partial tree).
|
||||
if _, err := h.execAsRoot(ctx, containerName, []string{
|
||||
"touch", v.markerPath(),
|
||||
}); err != nil {
|
||||
// Marker write failure with the new content already in place is a
|
||||
// weird state — content is fine on disk, but the plugin loader
|
||||
// will refuse to use it. Log loudly; do NOT roll back, since the
|
||||
// content is the latest, just unmarked. Operator can manually
|
||||
// `touch <plugin>/.complete` to recover.
|
||||
return fmt.Errorf("atomic install: write .complete marker (content landed but unmarked, manual recovery: touch %s): %w", v.markerPath(), err)
|
||||
}
|
||||
|
||||
// Step 5: GC — best-effort delete the previous snapshot. Failures here
|
||||
// just leave a directory; not load-bearing for correctness, the next
|
||||
// install or a separate sweeper will reclaim the space.
|
||||
if snapshotted {
|
||||
_, _ = h.execAsRoot(ctx, containerName, []string{
|
||||
"rm", "-rf", v.previousPath(),
|
||||
})
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// tarHostDirWithPrefix walks hostDir and writes a tar to a buffer with
|
||||
// every entry's name prefixed by `prefix`. Mirrors the prior streaming
|
||||
// shape used in copyPluginToContainer but with a configurable prefix
|
||||
// (the prior version hardcoded "plugins/<name>/"; we use a full
|
||||
// staging path so the extracted layout is the staging dir directly).
|
||||
//
|
||||
// Symlinks are skipped — same posture as streamDirAsTar elsewhere in
|
||||
// this file. Skipping prevents a hostile plugin from injecting a
|
||||
// symlink that, post-extract, points outside the plugin's own dir.
|
||||
func tarHostDirWithPrefix(hostDir, prefix string) (bytes.Buffer, error) {
|
||||
var buf bytes.Buffer
|
||||
tw := newTarWriter(&buf)
|
||||
defer tw.Close()
|
||||
if err := tarWalk(hostDir, prefix, tw); err != nil {
|
||||
return bytes.Buffer{}, err
|
||||
}
|
||||
return buf, nil
|
||||
}
|
||||
@@ -0,0 +1,70 @@
|
||||
package handlers
|
||||
|
||||
// plugins_atomic_tar.go — tar-walk helpers split out so the main atomic
|
||||
// install flow stays readable. The prefix argument lets the caller
|
||||
// arrange where the tar's contents land at extract time.
|
||||
|
||||
import (
|
||||
"archive/tar"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
)
|
||||
|
||||
// newTarWriter is a thin wrapper so atomic_test.go can swap the writer
|
||||
// destination if it needs to.
|
||||
func newTarWriter(w io.Writer) *tar.Writer {
|
||||
return tar.NewWriter(w)
|
||||
}
|
||||
|
||||
// tarWalk walks hostDir and writes every regular file + dir to the tar
|
||||
// writer with paths of the form `<prefix>/<relative>`. Symlinks are
|
||||
// skipped — same posture as streamDirAsTar in plugins_install_pipeline.go.
|
||||
//
|
||||
// The trailing-slash on prefix is normalized away: prefix "foo" and
|
||||
// prefix "foo/" produce identical archives.
|
||||
func tarWalk(hostDir, prefix string, tw *tar.Writer) error {
|
||||
prefix = filepath.Clean(prefix)
|
||||
return filepath.Walk(hostDir, func(p string, info os.FileInfo, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if info.Mode()&os.ModeSymlink != 0 {
|
||||
return nil // skip symlinks; see doc above
|
||||
}
|
||||
rel, err := filepath.Rel(hostDir, p)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if rel == "." {
|
||||
// Emit the prefix dir itself once, with the source dir's mode.
|
||||
hdr, err := tar.FileInfoHeader(info, "")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
hdr.Name = prefix + "/"
|
||||
return tw.WriteHeader(hdr)
|
||||
}
|
||||
hdr, err := tar.FileInfoHeader(info, "")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
hdr.Name = filepath.Join(prefix, rel)
|
||||
if info.IsDir() {
|
||||
hdr.Name += "/"
|
||||
}
|
||||
if err := tw.WriteHeader(hdr); err != nil {
|
||||
return err
|
||||
}
|
||||
if !info.Mode().IsRegular() {
|
||||
return nil
|
||||
}
|
||||
f, err := os.Open(p)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer f.Close()
|
||||
_, err = io.Copy(tw, f)
|
||||
return err
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,193 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"archive/tar"
|
||||
"bytes"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// TestInstallVersion_Paths: the path helpers must produce a stable shape
|
||||
// the in-container exec calls depend on. Pinning the layout here
|
||||
// catches a future refactor that accidentally changes where staging /
|
||||
// previous / live dirs live, which would break the swap atomicity.
|
||||
func TestInstallVersion_Paths(t *testing.T) {
|
||||
v := installVersion{plugin: "molecule-skill-foo", stamp: "20260508T141530Z"}
|
||||
|
||||
if got, want := v.stagedPath(), "/configs/plugins/.staging/molecule-skill-foo.20260508T141530Z"; got != want {
|
||||
t.Errorf("stagedPath = %q; want %q", got, want)
|
||||
}
|
||||
if got, want := v.previousPath(), "/configs/plugins/.previous/molecule-skill-foo.20260508T141530Z"; got != want {
|
||||
t.Errorf("previousPath = %q; want %q", got, want)
|
||||
}
|
||||
if got, want := v.livePath(), "/configs/plugins/molecule-skill-foo"; got != want {
|
||||
t.Errorf("livePath = %q; want %q", got, want)
|
||||
}
|
||||
if got, want := v.markerPath(), "/configs/plugins/molecule-skill-foo/.complete"; got != want {
|
||||
t.Errorf("markerPath = %q; want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestInstallVersion_StampUniqueness: two newInstallVersion calls within
|
||||
// the same second produce the same stamp (we use second precision); the
|
||||
// caller relies on the mv-rename being atomic, so collision-free
|
||||
// stamping is NOT a correctness requirement — but a regression that
|
||||
// changes stamp shape (e.g. RFC3339 with colons) would break the path
|
||||
// helpers since path.Join treats a colon as a regular char but ssh +
|
||||
// docker exec generally don't. Pin the no-colon shape.
|
||||
func TestInstallVersion_StampShape(t *testing.T) {
|
||||
v := newInstallVersion("anything")
|
||||
if strings.Contains(v.stamp, ":") {
|
||||
t.Errorf("stamp must not contain colons (breaks shell-quoting in exec): %q", v.stamp)
|
||||
}
|
||||
if strings.Contains(v.stamp, " ") {
|
||||
t.Errorf("stamp must not contain spaces: %q", v.stamp)
|
||||
}
|
||||
// Sanity: stamp parses as the documented format.
|
||||
if _, err := time.Parse("20060102T150405Z", v.stamp); err != nil {
|
||||
t.Errorf("stamp %q does not parse as 20060102T150405Z: %v", v.stamp, err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTarHostDirWithPrefix_HappyPath: walks a host dir, builds a tar with
|
||||
// the configured prefix, verifies every entry's name is rooted under
|
||||
// the prefix, and the file contents survive round-trip.
|
||||
func TestTarHostDirWithPrefix_HappyPath(t *testing.T) {
|
||||
hostDir := t.TempDir()
|
||||
|
||||
// Plant: <host>/plugin.yaml + <host>/skills/foo/SKILL.md + <host>/.complete
|
||||
files := map[string]string{
|
||||
"plugin.yaml": "name: foo\nversion: 1.0.0\n",
|
||||
"skills/foo/SKILL.md": "# Foo skill\n",
|
||||
".complete": "", // upstream may already have a marker
|
||||
}
|
||||
for rel, body := range files {
|
||||
full := filepath.Join(hostDir, rel)
|
||||
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(full, []byte(body), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
prefix := "configs/plugins/.staging/foo.20260508T141530Z"
|
||||
buf, err := tarHostDirWithPrefix(hostDir, prefix)
|
||||
if err != nil {
|
||||
t.Fatalf("tar: %v", err)
|
||||
}
|
||||
|
||||
// Read back the tar; collect names + body for regular files.
|
||||
got := map[string]string{}
|
||||
tr := tar.NewReader(&buf)
|
||||
for {
|
||||
hdr, err := tr.Next()
|
||||
if err == io.EOF {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatalf("tar reader: %v", err)
|
||||
}
|
||||
// Every entry must start with the prefix
|
||||
if !strings.HasPrefix(hdr.Name, prefix) {
|
||||
t.Errorf("entry %q does not start with prefix %q", hdr.Name, prefix)
|
||||
}
|
||||
if hdr.Typeflag == tar.TypeReg {
|
||||
body, err := io.ReadAll(tr)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
rel := strings.TrimPrefix(hdr.Name, prefix+"/")
|
||||
got[rel] = string(body)
|
||||
}
|
||||
}
|
||||
|
||||
for rel, want := range files {
|
||||
if got[rel] != want {
|
||||
t.Errorf("body[%q] = %q; want %q", rel, got[rel], want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestTarHostDirWithPrefix_SkipsSymlinks: a hostile plugin shouldn't be
|
||||
// able to ship a symlink that, post-extract, points outside its own
|
||||
// dir. The walker silently skips symlinks (same posture as
|
||||
// streamDirAsTar). Verify a planted symlink doesn't appear in the tar.
|
||||
func TestTarHostDirWithPrefix_SkipsSymlinks(t *testing.T) {
|
||||
hostDir := t.TempDir()
|
||||
// Plant a real file + a symlink pointing outside hostDir.
|
||||
if err := os.WriteFile(filepath.Join(hostDir, "real.txt"), []byte("ok"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
target := filepath.Join(t.TempDir(), "outside")
|
||||
if err := os.WriteFile(target, []byte("SHOULD NOT APPEAR"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Symlink(target, filepath.Join(hostDir, "evil")); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
buf, err := tarHostDirWithPrefix(hostDir, "p")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
names := []string{}
|
||||
tr := tar.NewReader(&buf)
|
||||
for {
|
||||
hdr, err := tr.Next()
|
||||
if err == io.EOF {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
names = append(names, hdr.Name)
|
||||
}
|
||||
sort.Strings(names)
|
||||
|
||||
for _, n := range names {
|
||||
if strings.Contains(n, "evil") {
|
||||
t.Errorf("symlink leaked into tar: %q", n)
|
||||
}
|
||||
}
|
||||
// real.txt should be present
|
||||
found := false
|
||||
for _, n := range names {
|
||||
if strings.HasSuffix(n, "real.txt") {
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Errorf("real.txt missing from tar; got names: %v", names)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTarHostDirWithPrefix_PrefixNormalization: trailing slash on prefix
|
||||
// should not change the archive shape. Pinning this so a future caller
|
||||
// passing "foo/" instead of "foo" doesn't double-slash entry names.
|
||||
func TestTarHostDirWithPrefix_PrefixNormalization(t *testing.T) {
|
||||
hostDir := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(hostDir, "x"), []byte("y"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
a, err := tarHostDirWithPrefix(hostDir, "foo")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
b, err := tarHostDirWithPrefix(hostDir, "foo/")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if !bytes.Equal(a.Bytes(), b.Bytes()) {
|
||||
t.Errorf("trailing-slash on prefix changed archive shape; tarHostDirWithPrefix should be slash-insensitive")
|
||||
}
|
||||
}
|
||||
@@ -276,7 +276,13 @@ func (h *PluginsHandler) resolveAndStage(ctx context.Context, req installRequest
|
||||
// using NewPluginsHandler without a DB; production wires it in router.go.
|
||||
func (h *PluginsHandler) deliverToContainer(ctx context.Context, workspaceID string, r *stageResult) error {
|
||||
if containerName := h.findRunningContainer(ctx, workspaceID); containerName != "" {
|
||||
if err := h.copyPluginToContainer(ctx, containerName, r.StagedDir, r.PluginName); err != nil {
|
||||
// Atomic stage→snapshot→swap→marker (molecule-core#114).
|
||||
// Replaces the prior single docker.CopyToContainer write that
|
||||
// left a partially-extracted tree on mid-install failure with
|
||||
// no rollback path. atomicCopyToContainer writes a .complete
|
||||
// marker as the last step; workspace-side plugin loaders should
|
||||
// refuse to load a plugin dir without it.
|
||||
if err := h.atomicCopyToContainer(ctx, containerName, r.StagedDir, r.PluginName); err != nil {
|
||||
log.Printf("Plugin install: failed to copy %s to %s: %v", r.PluginName, workspaceID, err)
|
||||
return newHTTPErr(http.StatusInternalServerError, gin.H{"error": "failed to copy plugin to container"})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user