diff --git a/cmd/server/main.go b/cmd/server/main.go index 04858923..0818a6f4 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -85,8 +85,9 @@ var ( // Multi-workflow mode flags databaseDSN = flag.String("database-dsn", "", "PostgreSQL connection string for multi-workflow mode") jwtSecret = flag.String("jwt-secret", "", "JWT signing secret for API authentication") - adminEmail = flag.String("admin-email", "", "Initial admin user email (first-run bootstrap)") - adminPassword = flag.String("admin-password", "", "Initial admin user password (first-run bootstrap)") + adminEmail = flag.String("admin-email", "", "Initial admin user email (first-run bootstrap)") + adminPassword = flag.String("admin-password", "", "Initial admin user password (first-run bootstrap)") + multiWorkflowAddr = flag.String("multi-workflow-addr", ":8081", "HTTP listen address (multi-workflow API)") // License flags licenseKey = flag.String("license-key", "", "License key for the workflow engine (or set WORKFLOW_LICENSE_KEY env var)") @@ -1196,10 +1197,29 @@ func (app *serverApp) importBundles(logger *slog.Logger) error { continue } + // Ensure the extracted workflow.yaml path is within the expected destination directory + absDestDir, absDestErr := filepath.Abs(destDir) + if absDestErr != nil { + logger.Error("Failed to resolve destination directory", "destDir", destDir, "error", absDestErr) + continue + } + + absWorkflowPath, absWorkflowErr := filepath.Abs(workflowPath) + if absWorkflowErr != nil { + logger.Error("Failed to resolve workflow path", "path", workflowPath, "error", absWorkflowErr) + continue + } + + rel, relErr := filepath.Rel(absDestDir, absWorkflowPath) + if relErr != nil || strings.HasPrefix(rel, "..") || filepath.IsAbs(rel) { + logger.Error("Workflow path is outside destination directory; possible path traversal", "path", absWorkflowPath, "destDir", absDestDir, "error", relErr) + continue + } + // Read the extracted workflow.yaml - yamlData, err := os.ReadFile(workflowPath) //nolint:gosec // G703: path from trusted bundle extraction + yamlData, err := os.ReadFile(absWorkflowPath) //nolint:gosec // G703: path validated to be within destDir if err != nil { - logger.Error("Failed to read workflow.yaml", "path", workflowPath, "error", err) + logger.Error("Failed to read workflow.yaml", "path", absWorkflowPath, "error", err) continue } yamlContent := string(yamlData) @@ -1325,10 +1345,14 @@ func run(ctx context.Context, app *serverApp, listenAddr string) error { // Clean up temp files and directories for _, f := range app.cleanupFiles { - os.Remove(f) //nolint:gosec // G703: cleaning up server-managed temp files + if err := os.Remove(f); err != nil && !os.IsNotExist(err) { //nolint:gosec // G703: cleaning up server-managed temp files + app.logger.Error("Temp file cleanup error", "path", f, "error", err) + } } for _, d := range app.cleanupDirs { - os.RemoveAll(d) //nolint:gosec // G703: cleaning up server-managed temp dirs + if err := os.RemoveAll(d); err != nil && !os.IsNotExist(err) { //nolint:gosec // G703: cleaning up server-managed temp dirs + app.logger.Error("Temp directory cleanup error", "path", d, "error", err) + } } return nil diff --git a/engine_structural_test.go b/engine_structural_test.go index 4e72a637..2f604ff3 100644 --- a/engine_structural_test.go +++ b/engine_structural_test.go @@ -14,6 +14,17 @@ import ( // --- Negative tests: core engine rejects unknown module types --- +// findPluginByName returns the plugin with the given name from allPlugins, +// or nil if no such plugin exists. +func findPluginByName(name string) plugin.EnginePlugin { + for _, p := range allPlugins() { + if p.Name() == name { + return p + } + } + return nil +} + // TestCoreRejectsUnknownModuleType verifies that the engine returns a clear // error when a config references a module type that no plugin provides. func TestCoreRejectsUnknownModuleType(t *testing.T) { @@ -201,7 +212,10 @@ func TestSelectivePluginLoading_HTTPOnly(t *testing.T) { engine := NewStdEngine(app, &mock.Logger{}) // Load only the HTTP plugin. - httpPlugin := allPlugins()[0] // http plugin is first + httpPlugin := findPluginByName("workflow-plugin-http") + if httpPlugin == nil { + t.Fatalf("HTTP plugin not found in allPlugins") + } if err := engine.LoadPlugin(httpPlugin); err != nil { t.Fatalf("LoadPlugin(http) failed: %v", err) } @@ -274,13 +288,28 @@ func TestEngineFactoryMapPopulatedByPlugins(t *testing.T) { "auth.jwt", } + // Build a set of known module types from the public schema API. + known := schema.KnownModuleTypes() + knownSet := make(map[string]bool, len(known)) + for _, k := range known { + knownSet[k] = true + } + for _, mt := range expectedTypes { - if _, ok := engine.moduleFactories[mt]; !ok { - t.Errorf("module type %q not found in factory map after loading all plugins", mt) + if !knownSet[mt] { + t.Errorf("module type %q not found in schema.KnownModuleTypes() after loading all plugins", mt) } } } +func getEngineModuleTypes(engine *StdEngine) []string { + types := make([]string, 0, len(engine.moduleFactories)) + for mt := range engine.moduleFactories { + types = append(types, mt) + } + return types +} + // TestSchemaKnowsPluginModuleTypes verifies that schema.RegisterModuleType is // called for each plugin's module types during LoadPlugin. func TestSchemaKnowsPluginModuleTypes(t *testing.T) { @@ -295,7 +324,7 @@ func TestSchemaKnowsPluginModuleTypes(t *testing.T) { } // Every module type in the factory map should be in the schema. - for mt := range engine.moduleFactories { + for _, mt := range getEngineModuleTypes(engine) { if !knownSet[mt] { t.Errorf("module type %q is in factory map but not in schema.KnownModuleTypes()", mt) } diff --git a/module/api_v1_test.go b/module/api_v1_test.go index 7bf27f8e..514f318c 100644 --- a/module/api_v1_test.go +++ b/module/api_v1_test.go @@ -29,6 +29,35 @@ func setupTestStore(t *testing.T) *V1Store { // --- V1Store Tests --- +func mustCreateCompany(t *testing.T, store *V1Store, name, desc, userID string) *V1Company { + t.Helper() + company, err := store.CreateCompany(name, desc, userID) + if err != nil { + t.Fatalf("CreateCompany(%q) failed: %v", name, err) + } + return company +} + +func mustCreateOrganization(t *testing.T, store *V1Store, companyID, name, desc, userID string) *V1Company { + t.Helper() + org, err := store.CreateOrganization(companyID, name, desc, userID) + if err != nil { + t.Fatalf("CreateOrganization(%q) failed: %v", name, err) + } + return org +} + +func mustCreateProject(t *testing.T, store *V1Store, orgID, name, desc, userID string) *V1Project { + t.Helper() + proj, err := store.CreateProject(orgID, name, desc, userID) + if err != nil { + t.Fatalf("CreateProject(%q) failed: %v", name, err) + } + return proj +} + +// --- V1Store Tests --- + func TestV1Store_CreateAndListCompanies(t *testing.T) { store := setupTestStore(t) @@ -116,8 +145,14 @@ func TestV1Store_CreateAndListOrganizations(t *testing.T) { func TestV1Store_CreateAndListProjects(t *testing.T) { store := setupTestStore(t) - company, _ := store.CreateCompany("Co", "", "u1") - org, _ := store.CreateOrganization(company.ID, "Org", "", "u1") + company, err := store.CreateCompany("Co", "", "u1") + if err != nil { + t.Fatalf("CreateCompany: %v", err) + } + org, err := store.CreateOrganization(company.ID, "Org", "", "u1") + if err != nil { + t.Fatalf("CreateOrganization: %v", err) + } p, err := store.CreateProject(org.ID, "My Project", "", "A cool project") if err != nil { @@ -151,9 +186,9 @@ func TestV1Store_CreateAndListProjects(t *testing.T) { func TestV1Store_WorkflowCRUD(t *testing.T) { store := setupTestStore(t) - company, _ := store.CreateCompany("Co", "", "u1") - org, _ := store.CreateOrganization(company.ID, "Org", "", "u1") - proj, _ := store.CreateProject(org.ID, "Proj", "", "") + company := mustCreateCompany(t, store, "Co", "", "u1") + org := mustCreateOrganization(t, store, company.ID, "Org", "", "u1") + proj := mustCreateProject(t, store, org.ID, "Proj", "", "") // Create wf, err := store.CreateWorkflow(proj.ID, "Test Workflow", "", "desc", "modules: []", "u1") @@ -221,7 +256,10 @@ func TestV1Store_WorkflowCRUD(t *testing.T) { t.Fatalf("DeleteWorkflow: %v", err) } - wfs, _ = store.ListWorkflows(proj.ID) + wfs, err = store.ListWorkflows(proj.ID) + if err != nil { + t.Fatalf("ListWorkflows after delete: %v", err) + } if len(wfs) != 0 { t.Errorf("got %d workflows after delete, want 0", len(wfs)) } @@ -230,15 +268,21 @@ func TestV1Store_WorkflowCRUD(t *testing.T) { func TestV1Store_WorkflowVersioning(t *testing.T) { store := setupTestStore(t) - company, _ := store.CreateCompany("Co", "", "u1") - org, _ := store.CreateOrganization(company.ID, "Org", "", "u1") - proj, _ := store.CreateProject(org.ID, "Proj", "", "") + company := mustCreateCompany(t, store, "Co", "", "u1") + org := mustCreateOrganization(t, store, company.ID, "Org", "", "u1") + proj := mustCreateProject(t, store, org.ID, "Proj", "", "") wf, _ := store.CreateWorkflow(proj.ID, "WF", "", "", "v1 config", "u1") // Update config 3 times to create versions 2, 3, 4 - store.UpdateWorkflow(wf.ID, "", "", "v2 config", "u1") - store.UpdateWorkflow(wf.ID, "", "", "v3 config", "u1") - store.UpdateWorkflow(wf.ID, "", "", "v4 config", "u1") + if _, err := store.UpdateWorkflow(wf.ID, "", "", "v2 config", "u1"); err != nil { + t.Fatalf("UpdateWorkflow v2: %v", err) + } + if _, err := store.UpdateWorkflow(wf.ID, "", "", "v3 config", "u1"); err != nil { + t.Fatalf("UpdateWorkflow v3: %v", err) + } + if _, err := store.UpdateWorkflow(wf.ID, "", "", "v4 config", "u1"); err != nil { + t.Fatalf("UpdateWorkflow v4: %v", err) + } versions, err := store.ListVersions(wf.ID) if err != nil { @@ -311,12 +355,18 @@ func TestV1Store_EnsureSystemHierarchy(t *testing.T) { if err != nil { t.Fatalf("EnsureSystemHierarchy (second call): %v", err) } + if c2 != companyID { + t.Errorf("expected same company ID %q, got %q", companyID, c2) + } + if o2 != orgID { + t.Errorf("expected same organization ID %q, got %q", orgID, o2) + } + if p2 != projectID { + t.Errorf("expected same project ID %q, got %q", projectID, p2) + } if w2 != workflowID { t.Errorf("expected same workflow ID %q, got %q", workflowID, w2) } - _ = c2 - _ = o2 - _ = p2 } func TestV1Store_ResetSystemWorkflow(t *testing.T) { diff --git a/plugins/admin/plugin_test.go b/plugins/admin/plugin_test.go index 4fddcbb1..c6d50230 100644 --- a/plugins/admin/plugin_test.go +++ b/plugins/admin/plugin_test.go @@ -9,7 +9,19 @@ import ( func TestPluginImplementsEnginePlugin(t *testing.T) { p := New() + // Compile-time assertion that *Plugin implements plugin.EnginePlugin. var _ plugin.EnginePlugin = p + + // Runtime checks via the EnginePlugin interface. + var ep plugin.EnginePlugin = p + + manifest := ep.EngineManifest() + if manifest == nil { + t.Fatal("EngineManifest() returned nil") + } + if err := manifest.Validate(); err != nil { + t.Fatalf("EngineManifest.Validate() failed: %v", err) + } } func TestPluginManifest(t *testing.T) { @@ -55,9 +67,14 @@ func TestModuleFactories(t *testing.T) { t.Errorf("missing factory for %q", typ) continue } - mod := factory("test-"+typ, map[string]any{}) + name := "test-" + typ + mod := factory(name, map[string]any{}) if mod == nil { t.Errorf("factory for %q returned nil", typ) + continue + } + if mod.Name() != name { + t.Errorf("factory for %q produced module with unexpected name: got %q, want %q", typ, mod.Name(), name) } } } @@ -141,6 +158,7 @@ func TestWiringHookMergesAdminConfig(t *testing.T) { }, } + originalLen := len(cfg.Modules) err := hooks[0].Hook(nil, cfg) if err != nil { t.Fatalf("wiring hook failed: %v", err) @@ -157,6 +175,11 @@ func TestWiringHookMergesAdminConfig(t *testing.T) { if !found { t.Error("admin-server module not found after wiring hook merge") } + + // Ensure that additional admin modules have been merged + if len(cfg.Modules) <= originalLen { + t.Errorf("expected modules to be added by wiring hook, before=%d after=%d", originalLen, len(cfg.Modules)) + } } func TestWiringHookSkipsIfAlreadyPresent(t *testing.T) {