forked from molecule-ai/molecule-core
Compare commits
12 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 8313b2a7a7 | |||
| 566c095571 | |||
| 694a036a7f | |||
| 8c1dbc6ba5 | |||
| 72d0d4b44e | |||
| 52e61d4704 | |||
| 10e510f50c | |||
| 6fac24e3de | |||
| f51722411b | |||
| f0015bff81 | |||
| b72d1d3f26 | |||
| c1de2287fd |
@@ -55,17 +55,8 @@ jobs:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
|
||||
- name: Checkout sibling plugin repo
|
||||
# Same reasoning as publish-workspace-server-image.yml — the Go
|
||||
# module's replace directive needs the plugin source so
|
||||
# CodeQL's "go build" phase can resolve.
|
||||
if: matrix.language == 'go'
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
repository: molecule-ai/molecule-ai-plugin-github-app-auth
|
||||
path: molecule-ai-plugin-github-app-auth
|
||||
token: ${{ secrets.PLUGIN_REPO_PAT || secrets.GITHUB_TOKEN }}
|
||||
|
||||
# github-app-auth sibling-checkout removed 2026-05-07 (#157):
|
||||
# plugin was dropped + the Dockerfile no longer needs it.
|
||||
# jq is pre-installed on ubuntu-latest — no setup step needed.
|
||||
|
||||
- name: Initialize CodeQL
|
||||
@@ -121,7 +112,7 @@ jobs:
|
||||
# 14-day retention — longer than default 3, short enough not
|
||||
# to bloat quota.
|
||||
if: always()
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
||||
uses: actions/upload-artifact@v3 # pinned to v3 for Gitea act_runner v0.6 compatibility (internal#46)
|
||||
with:
|
||||
name: codeql-sarif-${{ matrix.language }}
|
||||
path: sarif-results/${{ matrix.language }}/
|
||||
|
||||
@@ -95,16 +95,8 @@ jobs:
|
||||
- if: needs.detect-changes.outputs.run == 'true'
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
|
||||
- name: Checkout sibling plugin repo
|
||||
# Dockerfile.tenant copies molecule-ai-plugin-github-app-auth/
|
||||
# at the build-context root (see workspace-server/Dockerfile.tenant
|
||||
# line 19). PLUGIN_REPO_PAT pattern matches publish-workspace-server-image.yml.
|
||||
if: needs.detect-changes.outputs.run == 'true'
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
repository: molecule-ai/molecule-ai-plugin-github-app-auth
|
||||
path: molecule-ai-plugin-github-app-auth
|
||||
token: ${{ secrets.PLUGIN_REPO_PAT || secrets.GITHUB_TOKEN }}
|
||||
# github-app-auth sibling-checkout removed 2026-05-07 (#157):
|
||||
# the plugin was dropped + Dockerfile.tenant no longer COPYs it.
|
||||
|
||||
- name: Install Python deps for replays
|
||||
# peer-discovery-404 (and future replays) eval Python against the
|
||||
|
||||
@@ -60,8 +60,8 @@ permissions:
|
||||
packages: write
|
||||
|
||||
env:
|
||||
IMAGE_NAME: ghcr.io/molecule-ai/platform
|
||||
TENANT_IMAGE_NAME: ghcr.io/molecule-ai/platform-tenant
|
||||
IMAGE_NAME: 153263036946.dkr.ecr.us-east-2.amazonaws.com/molecule-ai/platform
|
||||
TENANT_IMAGE_NAME: 153263036946.dkr.ecr.us-east-2.amazonaws.com/molecule-ai/platform-tenant
|
||||
|
||||
jobs:
|
||||
build-and-push:
|
||||
@@ -70,31 +70,28 @@ jobs:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
|
||||
- name: Checkout sibling plugin repo
|
||||
# workspace-server/Dockerfile expects
|
||||
# ./molecule-ai-plugin-github-app-auth at build-context root because
|
||||
# the Go module has a `replace` directive pointing at /plugin inside
|
||||
# the image. Pre-repo-split the plugin lived in the monorepo; the
|
||||
# 2026-04-18 restructure moved it out but didn't add this clone step
|
||||
# — which is why publish was failing after that restructure.
|
||||
#
|
||||
# Uses a fine-grained PAT (PLUGIN_REPO_PAT) because the plugin repo
|
||||
# is private and the default GITHUB_TOKEN is scoped to THIS repo.
|
||||
# The PAT needs Contents:Read on molecule-ai/molecule-ai-plugin-
|
||||
# github-app-auth. Falls back to the default token for the (rare)
|
||||
# case where an operator made the plugin repo public.
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
repository: molecule-ai/molecule-ai-plugin-github-app-auth
|
||||
path: molecule-ai-plugin-github-app-auth
|
||||
token: ${{ secrets.PLUGIN_REPO_PAT || secrets.GITHUB_TOKEN }}
|
||||
# github-app-auth sibling-checkout removed 2026-05-07 (#157):
|
||||
# plugin was dropped + workspace-server/Dockerfile no longer
|
||||
# COPYs it.
|
||||
|
||||
- name: Log in to GHCR
|
||||
uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3
|
||||
- name: Configure AWS credentials for ECR
|
||||
# GHCR was the pre-suspension target; the molecule-ai org on
|
||||
# GitHub got swept 2026-05-06 and ghcr.io/molecule-ai/* is no
|
||||
# longer reachable. Post-suspension target is the operator's
|
||||
# ECR org (153263036946.dkr.ecr.us-east-2.amazonaws.com/
|
||||
# molecule-ai/*), which already hosts platform-tenant +
|
||||
# workspace-template-* + runner-base images. AWS creds come
|
||||
# from the AWS_ACCESS_KEY_ID/SECRET secrets bound to the
|
||||
# molecule-cp IAM user. Closes #161.
|
||||
uses: aws-actions/configure-aws-credentials@v4
|
||||
with:
|
||||
registry: ghcr.io
|
||||
username: ${{ github.actor }}
|
||||
password: ${{ secrets.GITHUB_TOKEN }}
|
||||
aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }}
|
||||
aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
|
||||
aws-region: us-east-2
|
||||
|
||||
- name: Log in to ECR
|
||||
id: ecr-login
|
||||
uses: aws-actions/amazon-ecr-login@v2
|
||||
|
||||
- name: Set up Docker Buildx
|
||||
uses: docker/setup-buildx-action@4d04d5d9486b7bd6fa91e7baf45bbb4f8b9deedd # v4.0.0
|
||||
@@ -184,3 +181,4 @@ jobs:
|
||||
org.opencontainers.image.source=https://github.com/${{ github.repository }}
|
||||
org.opencontainers.image.revision=${{ github.sha }}
|
||||
org.opencontainers.image.description=Molecule AI tenant platform + canvas — pending canary verify
|
||||
|
||||
|
||||
@@ -45,11 +45,18 @@ clone_category() {
|
||||
continue
|
||||
fi
|
||||
|
||||
echo " cloning $repo -> $target_dir/$name (ref=$ref)"
|
||||
# Post-2026-05-06 GitHub-org-suspension: clone from Gitea instead.
|
||||
# manifest.json paths still read "Molecule-AI/..." (the historic
|
||||
# github.com slug); Gitea lowercases the org part to "molecule-ai/".
|
||||
# Lowercase the org segment on the fly so we don't need to rewrite
|
||||
# every manifest entry.
|
||||
repo_gitea="$(echo "$repo" | awk -F/ '{ printf "%s", tolower($1); for (i=2; i<=NF; i++) printf "/%s", $i; print "" }')"
|
||||
|
||||
echo " cloning $repo_gitea -> $target_dir/$name (ref=$ref)"
|
||||
if [ "$ref" = "main" ]; then
|
||||
git clone --depth=1 -q "https://github.com/${repo}.git" "$target_dir/$name"
|
||||
git clone --depth=1 -q "https://git.moleculesai.app/${repo_gitea}.git" "$target_dir/$name"
|
||||
else
|
||||
git clone --depth=1 -q --branch "$ref" "https://github.com/${repo}.git" "$target_dir/$name"
|
||||
git clone --depth=1 -q --branch "$ref" "https://git.moleculesai.app/${repo_gitea}.git" "$target_dir/$name"
|
||||
fi
|
||||
CLONED=$((CLONED + 1))
|
||||
i=$((i + 1))
|
||||
|
||||
@@ -5,15 +5,11 @@
|
||||
|
||||
FROM golang:1.25-alpine AS builder
|
||||
WORKDIR /app
|
||||
# Plugin source for replace directive in go.mod
|
||||
COPY molecule-ai-plugin-github-app-auth/ /plugin/
|
||||
COPY workspace-server/go.mod workspace-server/go.sum ./
|
||||
# Add replace directives for Docker builds:
|
||||
# 1. Platform → plugin (plugin source at /plugin/)
|
||||
# 2. Plugin → platform (plugin's go.mod has a relative replace that doesn't
|
||||
# work in Docker; fix it to point at /app where the platform source lives)
|
||||
RUN echo 'replace github.com/Molecule-AI/molecule-ai-plugin-github-app-auth => /plugin' >> go.mod
|
||||
RUN sed -i 's|replace github.com/Molecule-AI/molecule-monorepo/platform => .*|replace github.com/Molecule-AI/molecule-monorepo/platform => /app|' /plugin/go.mod
|
||||
# github-app-auth plugin removed 2026-05-07 (#157): per-agent Gitea
|
||||
# identities replaced the GitHub-App-installation token flow after the
|
||||
# 2026-05-06 suspension. Pre-removal this stage COPY'd the sibling
|
||||
# plugin repo + injected a `replace` directive; both are gone.
|
||||
RUN go mod download
|
||||
COPY workspace-server/ .
|
||||
# GIT_SHA mirror of Dockerfile.tenant — see that file for the rationale.
|
||||
|
||||
@@ -16,9 +16,10 @@
|
||||
# ── Stage 1: Go platform binary ──────────────────────────────────────
|
||||
FROM golang:1.25-alpine AS go-builder
|
||||
WORKDIR /app
|
||||
COPY molecule-ai-plugin-github-app-auth/ /plugin/
|
||||
COPY workspace-server/go.mod workspace-server/go.sum ./
|
||||
RUN echo 'replace github.com/Molecule-AI/molecule-ai-plugin-github-app-auth => /plugin' >> go.mod
|
||||
# github-app-auth plugin removed 2026-05-07 (#157): per-agent Gitea
|
||||
# identities replaced GitHub-App tokens post-suspension. The sibling
|
||||
# COPY + replace directive are gone.
|
||||
RUN go mod download
|
||||
COPY workspace-server/ .
|
||||
|
||||
|
||||
@@ -30,8 +30,7 @@ import (
|
||||
|
||||
// External plugins — each registers EnvMutator(s) that run at workspace
|
||||
// provision time. Loaded via soft-dep gates in main() so self-hosters
|
||||
// without the App or without per-agent identity configured keep working.
|
||||
githubappauth "github.com/Molecule-AI/molecule-ai-plugin-github-app-auth/pluginloader"
|
||||
// without per-agent identity configured keep working.
|
||||
ghidentity "github.com/Molecule-AI/molecule-ai-plugin-gh-identity/pluginloader"
|
||||
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/pkg/provisionhook"
|
||||
@@ -180,12 +179,15 @@ func main() {
|
||||
}
|
||||
|
||||
// External-plugin env mutators — each plugin contributes 0+ mutators
|
||||
// onto a shared registry. Order matters: gh-identity populates
|
||||
// MOLECULE_AGENT_ROLE-derived attribution env vars that downstream
|
||||
// mutators and the workspace's install.sh can then read. Keep
|
||||
// github-app-auth last because it fails loudly on misconfig and its
|
||||
// failure mode is "no GITHUB_TOKEN" — worth surfacing after the
|
||||
// cheaper mutators already ran.
|
||||
// onto a shared registry. gh-identity populates MOLECULE_AGENT_ROLE-
|
||||
// derived attribution env vars that the workspace's install.sh can
|
||||
// then read.
|
||||
//
|
||||
// github-app-auth was dropped 2026-05-07 (closes #157): per-agent
|
||||
// Gitea identities (this gh-identity plugin's role-derived path)
|
||||
// replaced GitHub-App-installation tokens after the 2026-05-06
|
||||
// suspension. Workspaces now provision with a per-persona Gitea PAT
|
||||
// from .env instead of an App-rotated GITHUB_TOKEN.
|
||||
envReg := provisionhook.NewRegistry()
|
||||
|
||||
// gh-identity plugin — per-agent attribution via env injection + gh
|
||||
@@ -199,26 +201,6 @@ func main() {
|
||||
log.Printf("gh-identity: registered (config file=%q)", os.Getenv("MOLECULE_GH_IDENTITY_CONFIG_FILE"))
|
||||
}
|
||||
|
||||
// github-app-auth plugin — injects GITHUB_TOKEN + GH_TOKEN into every
|
||||
// workspace env using the App's installation access token (rotates ~hourly).
|
||||
// Soft-skip when GITHUB_APP_* env vars are absent so dev/self-hosters
|
||||
// without an App configured keep working; fail-loud only on MISCONFIG
|
||||
// (e.g. APP_ID set but key file missing), not on unset.
|
||||
if os.Getenv("GITHUB_APP_ID") != "" {
|
||||
if reg, err := githubappauth.BuildRegistry(); err != nil {
|
||||
log.Fatalf("github-app-auth plugin: %v", err)
|
||||
} else {
|
||||
// Copy the plugin's mutators onto the shared registry so the
|
||||
// TokenProvider probe (FirstTokenProvider) still finds them.
|
||||
for _, m := range reg.Mutators() {
|
||||
envReg.Register(m)
|
||||
}
|
||||
log.Printf("github-app-auth: registered, %d mutator(s) added to chain", reg.Len())
|
||||
}
|
||||
} else {
|
||||
log.Println("github-app-auth: GITHUB_APP_ID unset — skipping plugin registration (agents will use any PAT from .env)")
|
||||
}
|
||||
|
||||
wh.SetEnvMutators(envReg)
|
||||
log.Printf("env-mutator chain: %v", envReg.Names())
|
||||
|
||||
|
||||
@@ -5,7 +5,6 @@ go 1.25.0
|
||||
require (
|
||||
github.com/DATA-DOG/go-sqlmock v1.5.2
|
||||
github.com/Molecule-AI/molecule-ai-plugin-gh-identity v0.0.0-20260424033845-4fd5ac7be30f
|
||||
github.com/Molecule-AI/molecule-ai-plugin-github-app-auth v0.0.0-20260421064811-7d98ae51e31d
|
||||
github.com/alicebob/miniredis/v2 v2.37.0
|
||||
github.com/creack/pty v1.1.24
|
||||
github.com/docker/docker v28.5.2+incompatible
|
||||
|
||||
@@ -6,8 +6,6 @@ github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERo
|
||||
github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=
|
||||
github.com/Molecule-AI/molecule-ai-plugin-gh-identity v0.0.0-20260424033845-4fd5ac7be30f h1:YkLRhUg+9qr9OV9N8dG1Hj0Ml7TThHlRwh5F//oUJVs=
|
||||
github.com/Molecule-AI/molecule-ai-plugin-gh-identity v0.0.0-20260424033845-4fd5ac7be30f/go.mod h1:NqdtlWZDJvpXNJRHnMkPhTKHdA1LZTNH+63TB66JSOU=
|
||||
github.com/Molecule-AI/molecule-ai-plugin-github-app-auth v0.0.0-20260421064811-7d98ae51e31d h1:GpYhP6FxaJZc1Ljy5/YJ9ZIVGvfOqZBmDolNr2S5x2g=
|
||||
github.com/Molecule-AI/molecule-ai-plugin-github-app-auth v0.0.0-20260421064811-7d98ae51e31d/go.mod h1:3a6LR/zd7FjR9ZwLTbytwYlWuCBsbCOVFlEg0WnoYiM=
|
||||
github.com/alicebob/miniredis/v2 v2.37.0 h1:RheObYW32G1aiJIj81XVt78ZHJpHonHLHW7OLIshq68=
|
||||
github.com/alicebob/miniredis/v2 v2.37.0/go.mod h1:TcL7YfarKPGDAthEtl5NBeHZfeUQj6OXMm/+iu5cLMM=
|
||||
github.com/bsm/ginkgo/v2 v2.12.0 h1:Ny8MWAHyOepLGlLKYmXG4IEkioBysk6GpaRTLC8zwWs=
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"io"
|
||||
"log"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
@@ -177,16 +178,42 @@ func strDefault(m map[string]interface{}, key, fallback string) string {
|
||||
return fallback
|
||||
}
|
||||
|
||||
// findRunningContainer returns the live container name for workspaceID, or ""
|
||||
// when the container is genuinely not running OR the daemon errored
|
||||
// transiently. Routed through provisioner.RunningContainerName as the SSOT
|
||||
// (molecule-core#10) so this handler agrees with healthsweep on the same
|
||||
// inputs. Transient daemon errors are logged distinctly so triage doesn't
|
||||
// confuse a flaky daemon with a stopped container.
|
||||
func (h *PluginsHandler) findRunningContainer(ctx context.Context, workspaceID string) string {
|
||||
if h.docker == nil {
|
||||
name, err := provisioner.RunningContainerName(ctx, h.docker, workspaceID)
|
||||
if err != nil {
|
||||
log.Printf("plugins: docker inspect transient error for %s: %v (treating as not-running for this request)", workspaceID, err)
|
||||
return ""
|
||||
}
|
||||
name := provisioner.ContainerName(workspaceID)
|
||||
info, err := h.docker.ContainerInspect(ctx, name)
|
||||
if err == nil && info.State.Running {
|
||||
return name
|
||||
return name
|
||||
}
|
||||
|
||||
// isExternalRuntime reports whether the workspace's runtime is the
|
||||
// `external` (remote-pull) shape introduced in Phase 30. External
|
||||
// workspaces have no local container — `POST /plugins` (push-install via
|
||||
// docker exec) doesn't apply to them; they pull via the download endpoint
|
||||
// instead. Returns false (allow-install) if the lookup is unwired or
|
||||
// errors — failing open here is safe because the downstream
|
||||
// findRunningContainer step still gates on a real container being there.
|
||||
//
|
||||
// Background — molecule-core#10: without this check, external workspaces
|
||||
// fall through to findRunningContainer's NotFound path and return a
|
||||
// misleading 503 "container not running" instead of a clear "use the
|
||||
// pull endpoint" message.
|
||||
func (h *PluginsHandler) isExternalRuntime(workspaceID string) bool {
|
||||
if h.runtimeLookup == nil {
|
||||
return false
|
||||
}
|
||||
return ""
|
||||
runtime, err := h.runtimeLookup(workspaceID)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
return runtime == "external"
|
||||
}
|
||||
|
||||
func (h *PluginsHandler) execAsRoot(ctx context.Context, containerName string, cmd []string) (string, error) {
|
||||
|
||||
@@ -0,0 +1,176 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"go/ast"
|
||||
"go/parser"
|
||||
"go/token"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestFindRunningContainer_RoutesThroughProvisionerSSOT is a behavior-based
|
||||
// AST gate: it pins the invariant that PluginsHandler.findRunningContainer
|
||||
// MUST go through provisioner.RunningContainerName for its is-running check,
|
||||
// instead of carrying its own copy of cli.ContainerInspect logic.
|
||||
//
|
||||
// Background — molecule-core#10: a parallel impl of "is the workspace's
|
||||
// container running" used to live in plugins.go. It drifted from the
|
||||
// canonical impl in healthsweep (which goes through Provisioner.IsRunning
|
||||
// → RunningContainerName) on edge cases like "transient daemon error" —
|
||||
// the duplicate would 503 with a misleading message while healthsweep
|
||||
// correctly stayed defensive. Consolidating onto RunningContainerName as
|
||||
// the SSOT prevents any future copy from re-introducing that drift.
|
||||
//
|
||||
// Mutation invariant: if a future PR replaces the provisioner call with
|
||||
// `h.docker.ContainerInspect(...)` directly, this test fails. That's the
|
||||
// signal to either (a) extend RunningContainerName's contract OR (b)
|
||||
// document why this call site needs to differ. Either way: the drift
|
||||
// gets a reviewer's attention instead of shipping silently.
|
||||
func TestFindRunningContainer_RoutesThroughProvisionerSSOT(t *testing.T) {
|
||||
fset := token.NewFileSet()
|
||||
file, err := parser.ParseFile(fset, "plugins.go", nil, parser.ParseComments)
|
||||
if err != nil {
|
||||
t.Fatalf("parse plugins.go: %v", err)
|
||||
}
|
||||
|
||||
var fn *ast.FuncDecl
|
||||
ast.Inspect(file, func(n ast.Node) bool {
|
||||
f, ok := n.(*ast.FuncDecl)
|
||||
if !ok || f.Name.Name != "findRunningContainer" {
|
||||
return true
|
||||
}
|
||||
// Confirm receiver is *PluginsHandler so we don't pick up an unrelated
|
||||
// helper of the same name. ast.Recv is a FieldList — receivers carry
|
||||
// at most one field.
|
||||
if f.Recv == nil || len(f.Recv.List) == 0 {
|
||||
return true
|
||||
}
|
||||
fn = f
|
||||
return false
|
||||
})
|
||||
|
||||
if fn == nil {
|
||||
t.Fatal("findRunningContainer not found in plugins.go — was it renamed? update this test or the SSOT routing assumption")
|
||||
}
|
||||
|
||||
var (
|
||||
callsRunningContainerName bool
|
||||
callsContainerInspectRaw bool
|
||||
)
|
||||
ast.Inspect(fn.Body, func(n ast.Node) bool {
|
||||
call, ok := n.(*ast.CallExpr)
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
sel, ok := call.Fun.(*ast.SelectorExpr)
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
// Pkg.Func form: provisioner.RunningContainerName(...)
|
||||
if pkgIdent, ok := sel.X.(*ast.Ident); ok {
|
||||
if pkgIdent.Name == "provisioner" && sel.Sel.Name == "RunningContainerName" {
|
||||
callsRunningContainerName = true
|
||||
}
|
||||
}
|
||||
// Receiver-then-method form: h.docker.ContainerInspect(...) /
|
||||
// p.cli.ContainerInspect(...) — anything ending in
|
||||
// .ContainerInspect that's NOT routed through provisioner.
|
||||
if sel.Sel.Name == "ContainerInspect" {
|
||||
callsContainerInspectRaw = true
|
||||
}
|
||||
return true
|
||||
})
|
||||
|
||||
if !callsRunningContainerName {
|
||||
t.Errorf(
|
||||
"findRunningContainer must call provisioner.RunningContainerName for the SSOT inspect — see molecule-core#10. Found no such call.",
|
||||
)
|
||||
}
|
||||
if callsContainerInspectRaw {
|
||||
t.Errorf(
|
||||
"findRunningContainer carries a direct ContainerInspect call. This is the parallel-impl drift molecule-core#10 fixed. " +
|
||||
"Either route through provisioner.RunningContainerName OR — if a new use case truly needs a different inspect — extend RunningContainerName's contract first and update this gate to allow the specific delta.",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestProvisionerIsRunning_RoutesThroughRunningContainerName mirrors the
|
||||
// gate above but for the OTHER consumer of the SSOT — Provisioner.IsRunning
|
||||
// (called by healthsweep). If a future refactor makes IsRunning carry its
|
||||
// own ContainerInspect again, the two consumers' edge-case behaviors will
|
||||
// silently drift. Keep them yoked.
|
||||
func TestProvisionerIsRunning_RoutesThroughRunningContainerName(t *testing.T) {
|
||||
fset := token.NewFileSet()
|
||||
file, err := parser.ParseFile(fset, "../provisioner/provisioner.go", nil, parser.ParseComments)
|
||||
if err != nil {
|
||||
t.Fatalf("parse provisioner.go: %v", err)
|
||||
}
|
||||
|
||||
var fn *ast.FuncDecl
|
||||
ast.Inspect(file, func(n ast.Node) bool {
|
||||
f, ok := n.(*ast.FuncDecl)
|
||||
if !ok || f.Name.Name != "IsRunning" || f.Recv == nil {
|
||||
return true
|
||||
}
|
||||
// The receiver type must be *Provisioner specifically. CPProvisioner
|
||||
// has its own IsRunning that talks HTTP to the controlplane and is
|
||||
// out of scope for this gate.
|
||||
if !receiverIs(f, "Provisioner") {
|
||||
return true
|
||||
}
|
||||
fn = f
|
||||
return false
|
||||
})
|
||||
if fn == nil {
|
||||
t.Fatal("Provisioner.IsRunning not found — was it renamed? update this test")
|
||||
}
|
||||
|
||||
var (
|
||||
callsRunningContainerName bool
|
||||
callsContainerInspectRaw bool
|
||||
)
|
||||
ast.Inspect(fn.Body, func(n ast.Node) bool {
|
||||
call, ok := n.(*ast.CallExpr)
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
// Same-package call: bare identifier (e.g. RunningContainerName(...)).
|
||||
if id, ok := call.Fun.(*ast.Ident); ok && id.Name == "RunningContainerName" {
|
||||
callsRunningContainerName = true
|
||||
return true
|
||||
}
|
||||
// Selector call: pkg.Func (e.g. provisioner.RunningContainerName)
|
||||
// OR recv.Method (e.g. p.cli.ContainerInspect).
|
||||
sel, ok := call.Fun.(*ast.SelectorExpr)
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
switch sel.Sel.Name {
|
||||
case "RunningContainerName":
|
||||
callsRunningContainerName = true
|
||||
case "ContainerInspect":
|
||||
callsContainerInspectRaw = true
|
||||
}
|
||||
return true
|
||||
})
|
||||
|
||||
if !callsRunningContainerName {
|
||||
t.Errorf("Provisioner.IsRunning must call RunningContainerName for the SSOT inspect — see molecule-core#10")
|
||||
}
|
||||
if callsContainerInspectRaw {
|
||||
t.Errorf("Provisioner.IsRunning carries a direct ContainerInspect call; route through RunningContainerName instead")
|
||||
}
|
||||
}
|
||||
|
||||
// receiverIs reports whether fn's receiver is `*<typeName>` or `<typeName>`.
|
||||
func receiverIs(fn *ast.FuncDecl, typeName string) bool {
|
||||
if fn.Recv == nil || len(fn.Recv.List) == 0 {
|
||||
return false
|
||||
}
|
||||
expr := fn.Recv.List[0].Type
|
||||
if star, ok := expr.(*ast.StarExpr); ok {
|
||||
expr = star.X
|
||||
}
|
||||
id, ok := expr.(*ast.Ident)
|
||||
return ok && strings.EqualFold(id.Name, typeName)
|
||||
}
|
||||
@@ -32,6 +32,18 @@ import (
|
||||
// inside the workspace at startup.
|
||||
func (h *PluginsHandler) Install(c *gin.Context) {
|
||||
workspaceID := c.Param("id")
|
||||
// External-runtime guard (molecule-core#10): push-install via docker
|
||||
// exec is meaningless for `runtime='external'` workspaces — they have
|
||||
// no local container. Reject early with a hint pointing at the
|
||||
// pull-mode endpoint, instead of falling through to a misleading
|
||||
// "container not running" 503 from findRunningContainer.
|
||||
if h.isExternalRuntime(workspaceID) {
|
||||
c.JSON(http.StatusUnprocessableEntity, gin.H{
|
||||
"error": "plugin install via push is not supported for external runtimes",
|
||||
"hint": "external workspaces pull plugins via GET /workspaces/:id/plugins/:name/download",
|
||||
})
|
||||
return
|
||||
}
|
||||
// Cap the JSON body so a pathological POST can't exhaust parser memory.
|
||||
bodyMax := envx.Int64("PLUGIN_INSTALL_BODY_MAX_BYTES", defaultInstallBodyMaxBytes)
|
||||
c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, bodyMax)
|
||||
@@ -93,6 +105,16 @@ func (h *PluginsHandler) Uninstall(c *gin.Context) {
|
||||
pluginName := c.Param("name")
|
||||
ctx := c.Request.Context()
|
||||
|
||||
// Mirror Install's external-runtime guard (molecule-core#10) so the
|
||||
// two endpoints reject the same shape with the same message.
|
||||
if h.isExternalRuntime(workspaceID) {
|
||||
c.JSON(http.StatusUnprocessableEntity, gin.H{
|
||||
"error": "plugin uninstall via docker exec is not supported for external runtimes",
|
||||
"hint": "external workspaces manage their own plugin directory; remove it locally",
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
if err := validatePluginName(pluginName); err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid plugin name"})
|
||||
return
|
||||
|
||||
@@ -0,0 +1,176 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
// TestPluginInstall_ExternalRuntime_Returns422 — molecule-core#10.
|
||||
// Install on a `runtime='external'` workspace must NOT fall through to
|
||||
// findRunningContainer (which would 503 with a misleading "container not
|
||||
// running"). It must return 422 with a hint pointing at the pull-mode
|
||||
// download endpoint.
|
||||
func TestPluginInstall_ExternalRuntime_Returns422(t *testing.T) {
|
||||
h := NewPluginsHandler(t.TempDir(), nil, nil).
|
||||
WithRuntimeLookup(func(workspaceID string) (string, error) {
|
||||
return "external", nil
|
||||
})
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ba1789b0-4d21-4f4f-a878-fa226bf77cf5"}}
|
||||
c.Request = httptest.NewRequest(
|
||||
"POST",
|
||||
"/workspaces/ba1789b0-4d21-4f4f-a878-fa226bf77cf5/plugins",
|
||||
bytes.NewBufferString(`{"source":"local://my-plugin"}`),
|
||||
)
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.Install(c)
|
||||
|
||||
if w.Code != http.StatusUnprocessableEntity {
|
||||
t.Errorf("expected 422 (Unprocessable Entity) for runtime='external', got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), "external runtimes") {
|
||||
t.Errorf("expected error body to mention 'external runtimes', got: %s", w.Body.String())
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), "download") {
|
||||
t.Errorf("expected error body to point at the download endpoint, got: %s", w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestPluginUninstall_ExternalRuntime_Returns422 — symmetric guard on the
|
||||
// uninstall path (DELETE /workspaces/:id/plugins/:name). External
|
||||
// workspaces manage their own plugin directory locally; the platform
|
||||
// can't docker-exec into them.
|
||||
func TestPluginUninstall_ExternalRuntime_Returns422(t *testing.T) {
|
||||
h := NewPluginsHandler(t.TempDir(), nil, nil).
|
||||
WithRuntimeLookup(func(workspaceID string) (string, error) {
|
||||
return "external", nil
|
||||
})
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{
|
||||
{Key: "id", Value: "ba1789b0-4d21-4f4f-a878-fa226bf77cf5"},
|
||||
{Key: "name", Value: "my-plugin"},
|
||||
}
|
||||
c.Request = httptest.NewRequest(
|
||||
"DELETE",
|
||||
"/workspaces/ba1789b0-4d21-4f4f-a878-fa226bf77cf5/plugins/my-plugin",
|
||||
nil,
|
||||
)
|
||||
|
||||
h.Uninstall(c)
|
||||
|
||||
if w.Code != http.StatusUnprocessableEntity {
|
||||
t.Errorf("expected 422 for runtime='external', got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), "external runtimes") {
|
||||
t.Errorf("expected error body to mention 'external runtimes', got: %s", w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestPluginInstall_ContainerBackedRuntime_FallsThroughGuard — the runtime
|
||||
// guard MUST NOT short-circuit container-backed runtimes. With
|
||||
// `runtime='claude-code'` the install proceeds past the guard; without a
|
||||
// real plugin source it'll fail downstream (here: 404 from local resolver
|
||||
// because no plugin staged), which is the correct error to surface.
|
||||
//
|
||||
// This is the mutation-test partner: deleting the `runtime == "external"`
|
||||
// check would still pass TestPluginInstall_ExternalRuntime (because Install
|
||||
// would 404 instead of 422 — but the test asserts 422), and would still
|
||||
// pass this test (because both pre-fix and post-fix produce 404 here).
|
||||
// What this case pins is "non-external still falls through," catching
|
||||
// any over-eager guard that rejects all runtimes.
|
||||
func TestPluginInstall_ContainerBackedRuntime_FallsThroughGuard(t *testing.T) {
|
||||
h := NewPluginsHandler(t.TempDir(), nil, nil).
|
||||
WithRuntimeLookup(func(workspaceID string) (string, error) {
|
||||
return "claude-code", nil
|
||||
})
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "c7c28c0b-4ea5-4e75-9728-3ba860081708"}}
|
||||
c.Request = httptest.NewRequest(
|
||||
"POST",
|
||||
"/workspaces/c7c28c0b-4ea5-4e75-9728-3ba860081708/plugins",
|
||||
bytes.NewBufferString(`{"source":"local://nonexistent-plugin"}`),
|
||||
)
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.Install(c)
|
||||
|
||||
if w.Code == http.StatusUnprocessableEntity {
|
||||
t.Errorf("runtime='claude-code' must fall through the external guard; got 422: %s", w.Body.String())
|
||||
}
|
||||
// The local resolver will fail to find the plugin → 404. Anything
|
||||
// other than 422 (which would mean we mis-classified) is fine.
|
||||
if w.Code != http.StatusNotFound {
|
||||
t.Errorf("expected 404 (plugin not found in registry), got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestPluginInstall_NoRuntimeLookup_FailsOpen — when the runtime lookup
|
||||
// is unwired (test fixtures, niche deploy shapes) the guard MUST default
|
||||
// to allowing the install attempt. The downstream findRunningContainer
|
||||
// step still gates on a real container, so failing open here doesn't
|
||||
// expose a bypass — it just preserves backwards-compat with deployments
|
||||
// that haven't wired the lookup.
|
||||
func TestPluginInstall_NoRuntimeLookup_FailsOpen(t *testing.T) {
|
||||
h := NewPluginsHandler(t.TempDir(), nil, nil) // NO WithRuntimeLookup
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-no-lookup"}}
|
||||
c.Request = httptest.NewRequest(
|
||||
"POST",
|
||||
"/workspaces/ws-no-lookup/plugins",
|
||||
bytes.NewBufferString(`{"source":"local://nonexistent"}`),
|
||||
)
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.Install(c)
|
||||
|
||||
if w.Code == http.StatusUnprocessableEntity {
|
||||
t.Errorf("nil runtimeLookup must fall through (fail-open); got 422: %s", w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestPluginInstall_RuntimeLookupErrors_FailsOpen — same fail-open story
|
||||
// for transient DB errors in the lookup. We don't want a momentary
|
||||
// Postgres hiccup to flip every plugin install into a 422.
|
||||
func TestPluginInstall_RuntimeLookupErrors_FailsOpen(t *testing.T) {
|
||||
h := NewPluginsHandler(t.TempDir(), nil, nil).
|
||||
WithRuntimeLookup(func(workspaceID string) (string, error) {
|
||||
return "", errFakeDB
|
||||
})
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-db-flake"}}
|
||||
c.Request = httptest.NewRequest(
|
||||
"POST",
|
||||
"/workspaces/ws-db-flake/plugins",
|
||||
bytes.NewBufferString(`{"source":"local://nonexistent"}`),
|
||||
)
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.Install(c)
|
||||
|
||||
if w.Code == http.StatusUnprocessableEntity {
|
||||
t.Errorf("runtimeLookup error must fall through (fail-open); got 422: %s", w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// errFakeDB is a sentinel for the fail-open lookup-error case.
|
||||
var errFakeDB = &fakeError{msg: "synthetic db error"}
|
||||
|
||||
type fakeError struct{ msg string }
|
||||
|
||||
func (e *fakeError) Error() string { return e.msg }
|
||||
@@ -1073,18 +1073,53 @@ func (p *Provisioner) IsRunning(ctx context.Context, workspaceID string) (bool,
|
||||
if p == nil || p.cli == nil {
|
||||
return false, ErrNoBackend
|
||||
}
|
||||
name := ContainerName(workspaceID)
|
||||
info, err := p.cli.ContainerInspect(ctx, name)
|
||||
name, err := RunningContainerName(ctx, p.cli, workspaceID)
|
||||
if err != nil {
|
||||
if isContainerNotFound(err) {
|
||||
return false, nil
|
||||
}
|
||||
// Transient daemon error: caller treats !running as dead + restarts.
|
||||
// Returning true + the underlying error preserves the error for
|
||||
// metrics/logging without triggering the destructive path.
|
||||
return true, err
|
||||
}
|
||||
return info.State.Running, nil
|
||||
return name != "", nil
|
||||
}
|
||||
|
||||
// RunningContainerName returns the container name for workspaceID iff the
|
||||
// container exists AND is in the Running state. Single source of truth for
|
||||
// "what live container should I exec into for this workspace?" — used by
|
||||
// both Provisioner.IsRunning (healthsweep) and the plugins handler.
|
||||
//
|
||||
// Distinguishes three outcomes so callers can pick their own policy:
|
||||
//
|
||||
// - ("ws-<id>", nil): container is running. Caller can exec into it.
|
||||
// - ("", nil): container does not exist OR exists but is stopped
|
||||
// (NotFound, Exited, Created, Restarting…). Caller
|
||||
// should treat as a definitive "not running."
|
||||
// - ("", err): transient daemon error (timeout, socket EOF, ctx
|
||||
// cancel). Caller should NOT infer "not running" —
|
||||
// this could be a flaky daemon under load. Decide
|
||||
// per-callsite whether to fail soft or hard.
|
||||
//
|
||||
// Background — molecule-core#10: the plugins handler used to carry its own
|
||||
// copy of this inspect logic (`findRunningContainer`) which collapsed
|
||||
// transient errors into the same "" return as a genuinely-stopped container.
|
||||
// That hid daemon flakes as misleading 503 "container not running" responses
|
||||
// AND let the two impls drift on edge-case behavior. This is the SSOT.
|
||||
func RunningContainerName(ctx context.Context, cli *client.Client, workspaceID string) (string, error) {
|
||||
if cli == nil {
|
||||
return "", ErrNoBackend
|
||||
}
|
||||
name := ContainerName(workspaceID)
|
||||
info, err := cli.ContainerInspect(ctx, name)
|
||||
if err != nil {
|
||||
if isContainerNotFound(err) {
|
||||
return "", nil
|
||||
}
|
||||
return "", err
|
||||
}
|
||||
if info.State.Running {
|
||||
return name, nil
|
||||
}
|
||||
return "", nil
|
||||
}
|
||||
|
||||
// isContainerNotFound returns true when the Docker client indicates the
|
||||
|
||||
Reference in New Issue
Block a user