fix(ci): DSPX-4607 clear tests-bdd goconst, gosec, nestif and sloglint findings

tests-bdd/cukes/glue_platform.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"log"
 	"log/slog"
 	"net"
@@ -337,8 +338,8 @@ func (l *LocalDevPlatformGlue) Shutdown(platformCukesContext *PlatformTestSuiteC
 	if platformCukesContext.HasFailures || preserveTestDirectory {
 		platformCukesContext.Logger.Warn("preserving cukes directory for debugging",
 			slog.String("directory", l.Options.CukesDir),
-			slog.Bool("hasFailures", platformCukesContext.HasFailures),
-			slog.Bool("preserveOnFailure", preserveTestDirectory))
+			slog.Bool("has_failures", platformCukesContext.HasFailures),
+			slog.Bool("preserve_on_failure", preserveTestDirectory))
 		return nil
 	}
 
@@ -347,20 +348,49 @@ func (l *LocalDevPlatformGlue) Shutdown(platformCukesContext *PlatformTestSuiteC
 	return err
 }
 
+// changePermissions chmods every file below dirPath. The walk is scoped to an
+// os.Root so a symlink planted mid-walk cannot redirect the chmod outside dirPath.
 func changePermissions(dirPath string, mode os.FileMode) error {
-	err := filepath.Walk(dirPath, func(path string, info os.FileInfo, err error) error {
+	root, err := os.OpenRoot(dirPath)
+	if err != nil {
+		return err
+	}
+	defer root.Close()
+	return fs.WalkDir(root.FS(), ".", func(path string, entry fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
-		if !info.IsDir() {
-			err = os.Chmod(path, mode)
-			if err != nil {
-				return err
-			}
+		if entry.IsDir() {
+			return nil
+		}
+		return root.Chmod(path, mode)
+	})
+}
+
+// logKeyFiles dumps the generated PEM files so container startup failures can be
+// diagnosed from the test output. Scoped to an os.Root for the same reason as above.
+func logKeyFiles(keysDir string, logger *slog.Logger) error {
+	root, err := os.OpenRoot(keysDir)
+	if err != nil {
+		return err
+	}
+	defer root.Close()
+	return fs.WalkDir(root.FS(), ".", func(path string, _ fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if !strings.HasSuffix(path, ".pem") {
+			return nil
+		}
+		content, err := root.ReadFile(path)
+		if err != nil {
+			return err
 		}
+		logger.Warn("keys content",
+			slog.String("path", filepath.Join(keysDir, path)),
+			slog.String("content", string(content)))
 		return nil
 	})
-	return err
 }
 
 // Setup local platform including cert/key generation and platform and keycloak configuration setup.
@@ -396,7 +426,6 @@ func (l *LocalDevPlatformGlue) Setup(platformCukesContext *PlatformTestSuiteCont
 		return err
 	}
 
-	//nolint:nestif // refactor later - compose is private *dockercompose
 	if err := compose.WithEnv(map[string]string{
 		"POSTGRES_EXPOSE_PORT": strconv.Itoa(l.Options.postgresPort),
 		"KC_EXPOSE_PORT_HTTP":  strconv.Itoa(l.Options.keycloakPort), // Use HTTP port for BDD tests
@@ -405,22 +434,7 @@ func (l *LocalDevPlatformGlue) Setup(platformCukesContext *PlatformTestSuiteCont
 	}).Up(ctx, tc.Wait(true)); err != nil {
 		logger.Error("error standing up containers", slog.String("error", err.Error()))
 		// log key data
-		err := filepath.WalkDir(l.Options.KeysDir, func(path string, _ os.DirEntry, err error) error {
-			if err != nil {
-				return err
-			}
-			if strings.HasSuffix(path, ".pem") {
-				content, err := os.ReadFile(path)
-				if err != nil {
-					return err
-				}
-				logger.Warn("keys content",
-					slog.String("path", path),
-					slog.String("content", string(content)))
-			}
-			return nil
-		})
-		if err != nil {
+		if err := logKeyFiles(l.Options.KeysDir, logger); err != nil {
 			logger.Error("error dumping keys", slog.String("error", err.Error()))
 		}
 		LogComposeServices(compose, logger)

tests-bdd/cukes/glue_platform_test.go

@@ -0,0 +1,99 @@
+package cukes
+
+import (
+	"context"
+	"log/slog"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// recordingHandler captures records so tests can assert on emitted attributes.
+type recordingHandler struct {
+	records *[]slog.Record
+}
+
+func (recordingHandler) Enabled(context.Context, slog.Level) bool { return true }
+
+func (h recordingHandler) Handle(_ context.Context, r slog.Record) error {
+	*h.records = append(*h.records, r)
+	return nil
+}
+
+func (h recordingHandler) WithAttrs([]slog.Attr) slog.Handler { return h }
+
+func (h recordingHandler) WithGroup(string) slog.Handler { return h }
+
+func TestChangePermissions_RecursesFilesButNotDirs(t *testing.T) {
+	root := t.TempDir()
+	nested := filepath.Join(root, "sub", "deeper")
+	if err := os.MkdirAll(nested, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	files := []string{
+		filepath.Join(root, "top.pem"),
+		filepath.Join(root, "sub", "mid.pem"),
+		filepath.Join(nested, "leaf.pem"),
+	}
+	for _, f := range files {
+		if err := os.WriteFile(f, []byte("x"), 0o600); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	if err := changePermissions(root, 0o644); err != nil {
+		t.Fatalf("changePermissions: %v", err)
+	}
+
+	for _, f := range files {
+		info, err := os.Stat(f)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if got := info.Mode().Perm(); got != 0o644 {
+			t.Errorf("%s: got mode %o, want 644", f, got)
+		}
+	}
+	// Directories are skipped, so the 0700 created above must survive.
+	info, err := os.Stat(nested)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := info.Mode().Perm(); got != 0o700 {
+		t.Errorf("%s: directory mode changed to %o, want 700", nested, got)
+	}
+}
+
+func TestLogKeyFiles_OnlyReadsPEMs(t *testing.T) {
+	root := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(root, "sub"), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(root, "sub", "kas.pem"), []byte("PEMBODY"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(root, "notes.txt"), []byte("ignored"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	var logged []slog.Record
+	handler := recordingHandler{records: &logged}
+	if err := logKeyFiles(root, slog.New(handler)); err != nil {
+		t.Fatalf("logKeyFiles: %v", err)
+	}
+
+	if len(logged) != 1 {
+		t.Fatalf("got %d records, want 1 (only the .pem)", len(logged))
+	}
+	attrs := map[string]string{}
+	logged[0].Attrs(func(a slog.Attr) bool {
+		attrs[a.Key] = a.Value.String()
+		return true
+	})
+	if want := filepath.Join(root, "sub", "kas.pem"); attrs["path"] != want {
+		t.Errorf("path attr = %q, want %q", attrs["path"], want)
+	}
+	if attrs["content"] != "PEMBODY" {
+		t.Errorf("content attr = %q, want %q", attrs["content"], "PEMBODY")
+	}
+}

tests-bdd/cukes/steps_authorization.go

@@ -21,6 +21,9 @@ type AuthorizationServiceStepDefinitions struct{}
 
 const (
 	decisionResponse = "decisionResponse"
+	// singleResourceEphemeralID labels the lone resource in single-resource
+	// decision requests; the response is correlated back by this id.
+	singleResourceEphemeralID = "resource1"
 )
 
 func ConvertInterfaceToAny(jsonData []byte) (*anypb.Any, error) {
@@ -293,7 +296,7 @@ func (s *AuthorizationServiceStepDefinitions) sendDecisionRequestV2WithFulfillab
 			Name: strings.ToLower(action),
 		},
 		Resource: &authzV2.Resource{
-			EphemeralId: "resource1",
+			EphemeralId: singleResourceEphemeralID,
 			Resource: &authzV2.Resource_AttributeValues_{
 				AttributeValues: &authzV2.Resource_AttributeValues{
 					Fqns: resourceFQNs,
@@ -350,7 +353,7 @@ func (s *AuthorizationServiceStepDefinitions) iSendADecisionRequestForTokenForAc
 		},
 		Action: &policy.Action{Name: strings.ToLower(action)},
 		Resource: &authzV2.Resource{
-			EphemeralId: "resource1",
+			EphemeralId: singleResourceEphemeralID,
 			Resource: &authzV2.Resource_AttributeValues_{
 				AttributeValues: &authzV2.Resource_AttributeValues{Fqns: resourceFQNs},
 			},

tests-bdd/cukes/steps_localplatform.go

@@ -114,33 +114,39 @@ func (s *LocalPlatformStepDefinitions) aUser(ctx context.Context, username strin
 	return ctx, nil
 }
 
+// reattachSharedPlatform prepares a scenario that reuses a platform an earlier
+// @stateless scenario already stood up. Each godog scenario gets its own
+// PlatformScenarioContext with a nil SDK, so the SDK has to be re-attached to the
+// shared endpoint before step definitions (and the Background) can use it.
+func reattachSharedPlatform(ctx context.Context, scenarioContext *PlatformScenarioContext, options *platformStartOptions) error {
+	if scenarioContext.SDK == nil {
+		//nolint:contextcheck // otdf.New takes no context parameter
+		platformSDK, err := otdf.New(
+			scenarioContext.ScenarioOptions.PlatformEndpoint,
+			otdf.WithInsecureSkipVerifyConn(),
+			otdf.WithClientCredentials(clientID, platformClientSecret, nil),
+		)
+		if err != nil {
+			return err
+		}
+		scenarioContext.SDK = platformSDK
+	}
+	dbName := scenarioContext.ScenarioOptions.DatabaseName
+	if options.provisionDefaultPolicy && !scenarioContext.TestSuiteContext.defaultPolicyDBs[dbName] {
+		if err := provisionDefaultPolicy(ctx, scenarioContext.SDK); err != nil {
+			return fmt.Errorf("provision default policy: %w", err)
+		}
+		scenarioContext.TestSuiteContext.defaultPolicyDBs[dbName] = true
+	}
+	return nil
+}
+
 func (s *LocalPlatformStepDefinitions) commonLocalPlatform(ctx context.Context, options *platformStartOptions) (context.Context, error) {
 	scenarioContext := GetPlatformScenarioContext(ctx)
 	logger := scenarioContext.TestSuiteContext.Logger
 	if !scenarioContext.FirstScenario && scenarioContext.Stateless {
-		// Platform is already up (shared across @stateless scenarios), but
-		// each godog scenario gets its own PlatformScenarioContext with a
-		// nil SDK. Re-attach the SDK to the shared endpoint so step
-		// definitions (and the Background) keep working.
-		if scenarioContext.SDK == nil {
-			platformSDK, err := otdf.New(
-				scenarioContext.ScenarioOptions.PlatformEndpoint,
-				otdf.WithInsecureSkipVerifyConn(),
-				otdf.WithClientCredentials(clientID, platformClientSecret, nil),
-			)
-			if err != nil {
-				return ctx, err
-			}
-			scenarioContext.SDK = platformSDK
-		}
-		dbName := scenarioContext.ScenarioOptions.DatabaseName
-		if options.provisionDefaultPolicy && !scenarioContext.TestSuiteContext.defaultPolicyDBs[dbName] {
-			if err := provisionDefaultPolicy(ctx, scenarioContext.SDK); err != nil {
-				return ctx, fmt.Errorf("provision default policy: %w", err)
-			}
-			scenarioContext.TestSuiteContext.defaultPolicyDBs[dbName] = true
-		}
-		return ctx, nil
+		// Platform is already up (shared across @stateless scenarios).
+		return ctx, reattachSharedPlatform(ctx, scenarioContext, options)
 	}
 	localPlatformGlue, ok := (*scenarioContext.TestSuiteContext.PlatformGlue).(*LocalDevPlatformGlue)
 	if !ok {

tests-bdd/cukes/steps_registeredresources.go

@@ -199,7 +199,7 @@ func (s *RegisteredResourcesStepDefinitions) iSendADecisionRequestForEntityChain
 		},
 		Action: &policy.Action{Name: strings.ToLower(action)},
 		Resource: &authzV2.Resource{
-			EphemeralId: "resource1",
+			EphemeralId: singleResourceEphemeralID,
 			Resource: &authzV2.Resource_RegisteredResourceValueFqn{
 				RegisteredResourceValueFqn: resourceValueFQN,
 			},
@@ -238,7 +238,7 @@ func (s *RegisteredResourcesStepDefinitions) iSendADecisionRequestForRegisteredR
 		},
 		Action: &policy.Action{Name: strings.ToLower(action)},
 		Resource: &authzV2.Resource{
-			EphemeralId: "resource1",
+			EphemeralId: singleResourceEphemeralID,
 			Resource: &authzV2.Resource_AttributeValues_{
 				AttributeValues: &authzV2.Resource_AttributeValues{
 					Fqns: resourceFQNs,
@@ -278,7 +278,7 @@ func (s *RegisteredResourcesStepDefinitions) iSendADecisionRequestForRegisteredR
 		},
 		Action: &policy.Action{Name: strings.ToLower(action)},
 		Resource: &authzV2.Resource{
-			EphemeralId: "resource1",
+			EphemeralId: singleResourceEphemeralID,
 			Resource: &authzV2.Resource_RegisteredResourceValueFqn{
 				RegisteredResourceValueFqn: resourceFQN,
 			},