fix(org): add per-workspace RequiredEnv preflight check (#232) #251

Open
claude-ceo-assistant wants to merge 1 commits from test/issue-232-per-workspace-required-env-preflight into main
3 changed files with 304 additions and 0 deletions

View File

@ -697,6 +697,31 @@ func (h *OrgHandler) Import(c *gin.Context) {
})
return
}
// Per-workspace RequiredEnv preflight: checks that every RequiredEnv
// declared at the workspace level is covered by either (a) a global
// secret key (already validated above) or (b) a key present in the
// workspace's on-disk .env files (org root .env + per-workspace
// <files_dir>/.env). If neither covers the key the workspace is
// imported NOT CONFIGURED, which silently breaks the workspace at
// start time — the container boots without the required credential
// and every LLM call 401s or fails silently. Issue #232.
// orgBaseDir is empty when importing via body.Template (inline YAML);
// in that case we cannot check .env files, so we skip this check
// and fall back to the global-only gate above (which correctly
// rejects any strict requirement not covered by global_secrets).
if orgBaseDir != "" {
wsMissing := collectPerWorkspaceUnsatisfied(tmpl.Workspaces, orgBaseDir, configured)
if len(wsMissing) > 0 {
c.JSON(http.StatusPreconditionFailed, gin.H{
"error": "missing per-workspace required environment variables",
"missing_workspace_env": wsMissing,
"template": tmpl.Name,
"suggestion": "add these keys to the workspace's .env file or set them as global secrets before importing",
})
return
}
}
}
results := []map[string]interface{}{}

View File

@ -936,6 +936,59 @@ func flattenAndSortRequirements(by map[string]EnvRequirement) []EnvRequirement {
// can investigate.
const globalSecretsPreflightLimit = 10000
// PerWorkspaceUnsatisfied describes one per-workspace RequiredEnv that is
// not covered by either a global secret or a key present in the
// corresponding .env file.
type PerWorkspaceUnsatisfied struct {
Workspace string `json:"workspace"`
FilesDir string `json:"files_dir,omitempty"`
Unsatisfied EnvRequirement `json:"unsatisfied_env"`
}
// collectPerWorkspaceUnsatisfied recursively walks workspaces and returns
// per-workspace RequiredEnv entries that are not covered by (a) a global
// secret key or (b) a key present in the workspace's .env file(s) (org root
// .env + per-workspace <files_dir>/.env). This complements
// collectOrgEnv + loadConfiguredGlobalSecretKeys, which together only
// validate global-level RequiredEnv against global_secrets. The .env
// lookup mirrors the runtime resolution in createWorkspaceTree so that
// the preflight result matches what the container actually receives at
// start time.
func collectPerWorkspaceUnsatisfied(workspaces []OrgWorkspace, orgBaseDir string, globalSecrets map[string]struct{}) []PerWorkspaceUnsatisfied {
var out []PerWorkspaceUnsatisfied
var walk func([]OrgWorkspace)
walk = func(wsList []OrgWorkspace) {
for _, ws := range wsList {
// Build the set of keys available to this workspace from .env.
// This is the same three-source stack that createWorkspaceTree
// injects into the container:
// 1. Org root .env (parseEnvFile, no filesDir)
// 2. Workspace <files_dir>/.env (if filesDir is set)
// 3. Persona bootstrap env (MOLECULE_PERSONA_ROOT/<filesDir>/env)
// Items 1+2 are on-disk and testable; item 3 is host-only and
// skipped here (persona env does NOT satisfy required_env —
// it carries identity tokens, not workspace LLM keys).
envFromFiles := loadWorkspaceEnv(orgBaseDir, ws.FilesDir)
for _, req := range ws.RequiredEnv {
if req.IsSatisfied(globalSecrets) {
continue // covered by a global secret
}
if req.IsSatisfied(envFromFiles) {
continue // covered by a per-workspace .env file
}
out = append(out, PerWorkspaceUnsatisfied{
Workspace: ws.Name,
FilesDir: ws.FilesDir,
Unsatisfied: req,
})
}
walk(ws.Children)
}
}
walk(workspaces)
return out
}
func loadConfiguredGlobalSecretKeys(ctx context.Context) (map[string]struct{}, error) {
rows, err := db.DB.QueryContext(ctx,
`SELECT key FROM global_secrets WHERE octet_length(encrypted_value) > 0 LIMIT $1`,

View File

@ -0,0 +1,226 @@
package handlers
import (
"os"
"path/filepath"
"testing"
)
// TestCollectPerWorkspaceUnsatisfied_BothFiles covers the case where a key
// is present in both the org root .env and the workspace-specific .env. Both
// should satisfy the requirement (no entry in output).
func TestCollectPerWorkspaceUnsatisfied_BothFiles(t *testing.T) {
tmp := t.TempDir()
writeEnvFile(t, tmp, ".env", "PER_WS_KEY=globalvalue")
writeEnvFile(t, tmp, "ws-a/.env", "PER_WS_KEY=wsvalue")
workspaces := []OrgWorkspace{
{Name: "ws-a", FilesDir: "ws-a", RequiredEnv: []EnvRequirement{{Name: "PER_WS_KEY"}}},
}
// Global secret covers it.
globals := map[string]struct{}{"PER_WS_KEY": {}}
missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
if len(missing) != 0 {
t.Errorf("PER_WS_KEY present in global + .env: should be satisfied, got %d missing", len(missing))
}
}
// TestCollectPerWorkspaceUnsatisfied_WorkspaceEnvOnly covers a key present
// only in the workspace-specific .env file (not global). Should be satisfied.
func TestCollectPerWorkspaceUnsatisfied_WorkspaceEnvOnly(t *testing.T) {
tmp := t.TempDir()
writeEnvFile(t, tmp, "dev-lead/.env", "WORKSPACE_KEY=val")
workspaces := []OrgWorkspace{
{Name: "Dev Lead", FilesDir: "dev-lead", RequiredEnv: []EnvRequirement{{Name: "WORKSPACE_KEY"}}},
}
globals := map[string]struct{}{} // nothing in global
missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
if len(missing) != 0 {
t.Errorf("WORKSPACE_KEY in ws .env only: should be satisfied, got %d missing", len(missing))
}
}
// TestCollectPerWorkspaceUnsatisfied_OrgRootEnvOnly covers a key present
// only in the org root .env file (not per-workspace). Should be satisfied.
func TestCollectPerWorkspaceUnsatisfied_OrgRootEnvOnly(t *testing.T) {
tmp := t.TempDir()
writeEnvFile(t, tmp, ".env", "ORG_ROOT_KEY=val")
workspaces := []OrgWorkspace{
{Name: "ws-b", FilesDir: "ws-b", RequiredEnv: []EnvRequirement{{Name: "ORG_ROOT_KEY"}}},
}
globals := map[string]struct{}{}
missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
if len(missing) != 0 {
t.Errorf("ORG_ROOT_KEY in org root .env only: should be satisfied, got %d missing", len(missing))
}
}
// TestCollectPerWorkspaceUnsatisfied_GlobalCovers checks that a global
// secret alone satisfies a per-workspace RequiredEnv even when the .env
// files don't have the key.
func TestCollectPerWorkspaceUnsatisfied_GlobalCovers(t *testing.T) {
tmp := t.TempDir()
// No .env files at all.
workspaces := []OrgWorkspace{
{Name: "ws-c", RequiredEnv: []EnvRequirement{{Name: "GLOBAL_COVERED"}}},
}
globals := map[string]struct{}{"GLOBAL_COVERED": {}}
missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
if len(missing) != 0 {
t.Errorf("GLOBAL_COVERED satisfied by global: should be satisfied, got %d missing", len(missing))
}
}
// TestCollectPerWorkspaceUnsatisfied_Missing covers the core bug: a
// RequiredEnv declared at the workspace level where the key is absent from
// both global_secrets and the .env file. The import MUST return 412.
func TestCollectPerWorkspaceUnsatisfied_Missing(t *testing.T) {
tmp := t.TempDir()
// No .env files at all.
workspaces := []OrgWorkspace{
{Name: "Dev Lead", FilesDir: "dev-lead", RequiredEnv: []EnvRequirement{{Name: "MISSING_REQUIRED_KEY"}}},
}
globals := map[string]struct{}{} // no global secret
missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
if len(missing) != 1 {
t.Fatalf("expected 1 missing entry, got %d", len(missing))
}
if missing[0].Workspace != "Dev Lead" {
t.Errorf("expected workspace 'Dev Lead', got %q", missing[0].Workspace)
}
if missing[0].Unsatisfied.Name != "MISSING_REQUIRED_KEY" {
t.Errorf("expected unsatisfied key 'MISSING_REQUIRED_KEY', got %q", missing[0].Unsatisfied.Name)
}
if missing[0].FilesDir != "dev-lead" {
t.Errorf("expected files_dir 'dev-lead', got %q", missing[0].FilesDir)
}
}
// TestCollectPerWorkspaceUnsatisfied_AnyOfGroup covers an any-of group where
// none of the alternatives are present in global or .env. Should report
// the group as unsatisfied.
func TestCollectPerWorkspaceUnsatisfied_AnyOfGroup(t *testing.T) {
tmp := t.TempDir()
workspaces := []OrgWorkspace{
{
Name: "Claude Bot",
FilesDir: "claude-bot",
RequiredEnv: []EnvRequirement{
{AnyOf: []string{"ANTHROPIC_API_KEY", "CLAUDE_CODE_OAUTH_TOKEN"}},
},
},
}
globals := map[string]struct{}{}
missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
if len(missing) != 1 {
t.Fatalf("expected 1 missing any-of entry, got %d", len(missing))
}
if missing[0].Workspace != "Claude Bot" {
t.Errorf("expected workspace 'Claude Bot', got %q", missing[0].Workspace)
}
if len(missing[0].Unsatisfied.AnyOf) != 2 {
t.Errorf("expected any-of group with 2 members, got %v", missing[0].Unsatisfied.AnyOf)
}
}
// TestCollectPerWorkspaceUnsatisfied_NestedChildren covers grandchildren
// workspaces that also declare RequiredEnv. The recursive walk must visit
// children and grandchildren.
func TestCollectPerWorkspaceUnsatisfied_NestedChildren(t *testing.T) {
tmp := t.TempDir()
workspaces := []OrgWorkspace{
{
Name: "Root",
Children: []OrgWorkspace{
{
Name: "Child",
Children: []OrgWorkspace{
{Name: "Grandchild", FilesDir: "grandchild", RequiredEnv: []EnvRequirement{{Name: "DEEP_KEY"}}},
},
},
},
},
}
globals := map[string]struct{}{}
missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
if len(missing) != 1 {
t.Fatalf("expected 1 missing entry from grandchild, got %d", len(missing))
}
if missing[0].Workspace != "Grandchild" {
t.Errorf("expected 'Grandchild', got %q", missing[0].Workspace)
}
}
// TestCollectPerWorkspaceUnsatisfied_EmptyOrgBaseDir covers the case where
// orgBaseDir is empty (inline template import). No .env files can be
// checked, so missing keys cannot be attributed to .env absence. The
// function should NOT crash and should only report entries satisfiable
// by global (all missing since globals is empty).
func TestCollectPerWorkspaceUnsatisfied_EmptyOrgBaseDir(t *testing.T) {
workspaces := []OrgWorkspace{
{Name: "ws-x", RequiredEnv: []EnvRequirement{{Name: "KEY_X"}}},
}
globals := map[string]struct{}{}
missing := collectPerWorkspaceUnsatisfied(workspaces, "", globals)
// With no orgBaseDir and no global, KEY_X must be reported missing.
if len(missing) != 1 {
t.Errorf("expected 1 missing with empty orgBaseDir, got %d", len(missing))
}
}
// TestCollectPerWorkspaceUnsatisfied_MultipleWorkspaces reports only the
// workspace whose RequiredEnv is unsatisfied, not the whole batch.
func TestCollectPerWorkspaceUnsatisfied_MultipleWorkspaces(t *testing.T) {
tmp := t.TempDir()
writeEnvFile(t, tmp, "ws-ok/.env", "OK_KEY=val")
workspaces := []OrgWorkspace{
{Name: "ws-ok", FilesDir: "ws-ok", RequiredEnv: []EnvRequirement{{Name: "OK_KEY"}}},
{Name: "ws-missing", FilesDir: "ws-missing", RequiredEnv: []EnvRequirement{{Name: "BAD_KEY"}}},
}
globals := map[string]struct{}{}
missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
if len(missing) != 1 {
t.Errorf("expected exactly 1 missing (BAD_KEY), got %d", len(missing))
}
if missing[0].Workspace != "ws-missing" {
t.Errorf("expected missing workspace 'ws-missing', got %q", missing[0].Workspace)
}
}
// writeEnvFile is a test helper that creates a .env file at the given path
// with the given content.
func writeEnvFile(t *testing.T, baseDir, relPath, content string) {
t.Helper()
fullPath := filepath.Join(baseDir, relPath)
if err := os.MkdirAll(filepath.Dir(fullPath), 0755); err != nil {
t.Fatalf("mkdirAll: %v", err)
}
if err := os.WriteFile(fullPath, []byte(content), 0644); err != nil {
t.Fatalf("writeFile %s: %v", fullPath, err)
}
}