From a26e7d279c26c79ec03e843c633821abcabce637 Mon Sep 17 00:00:00 2001 From: James Salt Date: Wed, 3 Jun 2026 11:45:13 +0100 Subject: [PATCH 1/3] BCH-1290: Document tar.gz bundle gap; handle ENOTDIR in LoadBundleRootData When the agent passes a tar.gz file path as policyPath, path-joining into a file returns ENOTDIR rather than ErrNotExist. Handle both as "not found" so the function does not propagate an error for non-directory candidates. Add TestLoadBundleRootData_TarGzBundle to document the remaining gap: the function silently returns empty defaults for tar.gz paths because it cannot read data.json from inside the archive. Policies relying on data.* will receive undefined references until tar.gz extraction support is added. Co-Authored-By: Claude Sonnet 4.6 --- internal/eval.go | 3 ++- internal/eval_test.go | 48 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/internal/eval.go b/internal/eval.go index 9fe74b2..a4cea4d 100644 --- a/internal/eval.go +++ b/internal/eval.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "strings" + "syscall" policyManager "github.com/compliance-framework/agent/policy-manager" "github.com/compliance-framework/agent/runner/proto" @@ -165,7 +166,7 @@ func LoadBundleRootData(policyPath string, overrides map[string]interface{}) (ma } for _, p := range candidates { raw, err := os.ReadFile(p) - if errors.Is(err, os.ErrNotExist) { + if errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { continue } if err != nil { diff --git a/internal/eval_test.go b/internal/eval_test.go index 1edadb8..dd2f3d9 100644 --- a/internal/eval_test.go +++ b/internal/eval_test.go @@ -1,11 +1,59 @@ package internal import ( + "archive/tar" + "compress/gzip" "os" "path/filepath" "testing" ) +// writeTarGz creates a tar.gz archive at dest containing a single file at +// the given internal path with the given content. +func writeTarGz(t *testing.T, dest, internalPath, content string) { + t.Helper() + f, err := os.Create(dest) + if err != nil { + t.Fatal(err) + } + defer f.Close() + gw := gzip.NewWriter(f) + defer gw.Close() + tw := tar.NewWriter(gw) + defer tw.Close() + body := []byte(content) + if err := tw.WriteHeader(&tar.Header{Name: internalPath, Mode: 0644, Size: int64(len(body))}); err != nil { + t.Fatal(err) + } + if _, err := tw.Write(body); err != nil { + t.Fatal(err) + } +} + +// TestLoadBundleRootData_TarGzBundle documents the known gap: when the agent +// supplies a bundle as a tar.gz file path, LoadBundleRootData receives that +// file path as policyPath. Path-joining into a file returns ENOTDIR (treated +// as not-found and skipped), and data.json inside the archive is never read. +// Bundle defaults are silently lost; policies relying on data.* will fail. +// Fix: detect tar.gz paths in LoadBundleRootData and extract data.json from +// the archive before falling back to the filesystem candidates. +func TestLoadBundleRootData_TarGzBundle(t *testing.T) { + root := t.TempDir() + bundlePath := filepath.Join(root, "bundle.tar.gz") + writeTarGz(t, bundlePath, "data.json", `{"expiry_warning_days":30}`) + + result, err := LoadBundleRootData(bundlePath, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // data.json is not extracted from the archive; bundle defaults are silently lost. + // Update this assertion once tar.gz support is implemented. + if _, loaded := result["expiry_warning_days"]; loaded { + t.Fatal("tar.gz bundle support appears to be implemented — update this test to assert correct loading behaviour") + } +} + func TestLoadBundleRootData_OverridesWinOverBundleDefaults(t *testing.T) { dir := t.TempDir() bundleJSON := `{"expiry_warning_days":30,"required_certificate_tags":["Environment"]}` From c68ce3209ee464f394d075ff03d8dc231a49114a Mon Sep 17 00:00:00 2001 From: James Salt Date: Wed, 3 Jun 2026 11:47:19 +0100 Subject: [PATCH 2/3] BCH-1290: Support reading data.json from tar.gz OPA bundles When the agent supplies a bundle as a tar.gz file path, LoadBundleRootData now detects the .tar.gz suffix and extracts data.json from the archive rather than attempting (and failing) filesystem path-joins into a file. The merge semantics are unchanged: bundle defaults are loaded first, operator-supplied overrides win on conflict. Tests: flip the gap test to assert correct loading; add TestLoadBundleRootData_TarGzBundleOverridesWin to confirm operator overrides still take precedence over archive defaults. Co-Authored-By: Claude Sonnet 4.6 --- internal/eval.go | 53 +++++++++++++++++++++++++++++++++++++++++++ internal/eval_test.go | 34 +++++++++++++++++---------- 2 files changed, 75 insertions(+), 12 deletions(-) diff --git a/internal/eval.go b/internal/eval.go index a4cea4d..cadcacc 100644 --- a/internal/eval.go +++ b/internal/eval.go @@ -1,10 +1,13 @@ package internal import ( + "archive/tar" + "compress/gzip" "context" "encoding/json" "errors" "fmt" + "io" "os" "path/filepath" "strings" @@ -160,6 +163,10 @@ func certificateBaseLabels() map[string]string { // policies/ directory itself, so both locations are checked. overrides win on // conflict, so operator-supplied policy_data takes precedence over bundle defaults. func LoadBundleRootData(policyPath string, overrides map[string]interface{}) (map[string]interface{}, error) { + if strings.HasSuffix(policyPath, ".tar.gz") { + return loadDataFromTarGz(policyPath, overrides) + } + candidates := []string{ filepath.Join(policyPath, "data.json"), filepath.Join(filepath.Dir(policyPath), "data.json"), @@ -188,6 +195,52 @@ func LoadBundleRootData(policyPath string, overrides map[string]interface{}) (ma return overrides, nil } +// loadDataFromTarGz reads data.json from the root of a tar.gz OPA bundle and +// merges it with overrides (overrides win on conflict). +func loadDataFromTarGz(archivePath string, overrides map[string]interface{}) (map[string]interface{}, error) { + f, err := os.Open(archivePath) + if err != nil { + return nil, fmt.Errorf("opening bundle %s: %w", archivePath, err) + } + defer f.Close() + + gr, err := gzip.NewReader(f) + if err != nil { + return nil, fmt.Errorf("reading gzip from bundle %s: %w", archivePath, err) + } + defer gr.Close() + + tr := tar.NewReader(gr) + for { + hdr, err := tr.Next() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return nil, fmt.Errorf("reading tar from bundle %s: %w", archivePath, err) + } + if strings.TrimPrefix(hdr.Name, "./") == "data.json" { + raw, err := io.ReadAll(tr) + if err != nil { + return nil, fmt.Errorf("reading data.json from bundle %s: %w", archivePath, err) + } + var bundleData map[string]interface{} + if err := json.Unmarshal(raw, &bundleData); err != nil { + return nil, fmt.Errorf("parsing data.json from bundle %s: %w", archivePath, err) + } + merged := make(map[string]interface{}, len(bundleData)+len(overrides)) + for k, v := range bundleData { + merged[k] = v + } + for k, v := range overrides { + merged[k] = v + } + return merged, nil + } + } + return overrides, nil +} + // arnCertID extracts the certificate UUID from an ACM ARN. // ARN format: arn:aws:acm:::certificate/ func arnCertID(arn string) string { diff --git a/internal/eval_test.go b/internal/eval_test.go index dd2f3d9..92fb669 100644 --- a/internal/eval_test.go +++ b/internal/eval_test.go @@ -30,27 +30,37 @@ func writeTarGz(t *testing.T, dest, internalPath, content string) { } } -// TestLoadBundleRootData_TarGzBundle documents the known gap: when the agent -// supplies a bundle as a tar.gz file path, LoadBundleRootData receives that -// file path as policyPath. Path-joining into a file returns ENOTDIR (treated -// as not-found and skipped), and data.json inside the archive is never read. -// Bundle defaults are silently lost; policies relying on data.* will fail. -// Fix: detect tar.gz paths in LoadBundleRootData and extract data.json from -// the archive before falling back to the filesystem candidates. func TestLoadBundleRootData_TarGzBundle(t *testing.T) { root := t.TempDir() bundlePath := filepath.Join(root, "bundle.tar.gz") - writeTarGz(t, bundlePath, "data.json", `{"expiry_warning_days":30}`) + writeTarGz(t, bundlePath, "data.json", `{"expiry_warning_days":30,"required_certificate_tags":["Environment"]}`) result, err := LoadBundleRootData(bundlePath, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } - // data.json is not extracted from the archive; bundle defaults are silently lost. - // Update this assertion once tar.gz support is implemented. - if _, loaded := result["expiry_warning_days"]; loaded { - t.Fatal("tar.gz bundle support appears to be implemented — update this test to assert correct loading behaviour") + if got := result["expiry_warning_days"]; got != float64(30) { + t.Errorf("expiry_warning_days: got %v, want 30", got) + } + if _, ok := result["required_certificate_tags"]; !ok { + t.Error("required_certificate_tags from tar.gz bundle missing from result") + } +} + +func TestLoadBundleRootData_TarGzBundleOverridesWin(t *testing.T) { + root := t.TempDir() + bundlePath := filepath.Join(root, "bundle.tar.gz") + writeTarGz(t, bundlePath, "data.json", `{"expiry_warning_days":30}`) + + overrides := map[string]interface{}{"expiry_warning_days": float64(90)} + result, err := LoadBundleRootData(bundlePath, overrides) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := result["expiry_warning_days"]; got != float64(90) { + t.Errorf("expiry_warning_days: got %v, want 90 (operator override must win)", got) } } From 8cb9c73e7e20279ce3f8c4d70e7d92a049cdf36b Mon Sep 17 00:00:00 2001 From: James Salt Date: Wed, 3 Jun 2026 11:56:12 +0100 Subject: [PATCH 3/3] =?UTF-8?q?BCH-1290:=20Remove=20LoadBundleRootData=20?= =?UTF-8?q?=E2=80=94=20OPA=20loads=20data.json=20natively?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rego.LoadBundle (used by the policy manager's GenerateResults) handles data.json loading from both directories and tar.gz bundles as a first- class feature. The plugin's LoadBundleRootData and loadDataFromTarGz were duplicating this work and failing to handle non-.tar.gz archive formats. Removing them reduces complexity and delegates bundle handling entirely to OPA. Operator-supplied policy_data overrides are still applied on top via the policy manager's writePolicyData after the bundle is loaded. Co-Authored-By: Claude Sonnet 4.6 --- internal/eval.go | 92 ------------------------ internal/eval_test.go | 160 ------------------------------------------ main.go | 13 +--- 3 files changed, 1 insertion(+), 264 deletions(-) delete mode 100644 internal/eval_test.go diff --git a/internal/eval.go b/internal/eval.go index cadcacc..31ac5bf 100644 --- a/internal/eval.go +++ b/internal/eval.go @@ -1,17 +1,10 @@ package internal import ( - "archive/tar" - "compress/gzip" "context" - "encoding/json" "errors" "fmt" - "io" - "os" - "path/filepath" "strings" - "syscall" policyManager "github.com/compliance-framework/agent/policy-manager" "github.com/compliance-framework/agent/runner/proto" @@ -156,91 +149,6 @@ func certificateBaseLabels() map[string]string { } } -// LoadBundleRootData reads data.json from the OPA bundle root and merges it -// with overrides. When the agent downloads a policy OCI artifact it returns the -// policies/ subdirectory as policyPath; the bundle's data.json lives one level -// up in the bundle root. For local source trees the data.json lives inside the -// policies/ directory itself, so both locations are checked. overrides win on -// conflict, so operator-supplied policy_data takes precedence over bundle defaults. -func LoadBundleRootData(policyPath string, overrides map[string]interface{}) (map[string]interface{}, error) { - if strings.HasSuffix(policyPath, ".tar.gz") { - return loadDataFromTarGz(policyPath, overrides) - } - - candidates := []string{ - filepath.Join(policyPath, "data.json"), - filepath.Join(filepath.Dir(policyPath), "data.json"), - } - for _, p := range candidates { - raw, err := os.ReadFile(p) - if errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { - continue - } - if err != nil { - return nil, fmt.Errorf("reading bundle data %s: %w", p, err) - } - var bundleData map[string]interface{} - if err := json.Unmarshal(raw, &bundleData); err != nil { - return nil, fmt.Errorf("parsing bundle data %s: %w", p, err) - } - merged := make(map[string]interface{}, len(bundleData)+len(overrides)) - for k, v := range bundleData { - merged[k] = v - } - for k, v := range overrides { - merged[k] = v - } - return merged, nil - } - return overrides, nil -} - -// loadDataFromTarGz reads data.json from the root of a tar.gz OPA bundle and -// merges it with overrides (overrides win on conflict). -func loadDataFromTarGz(archivePath string, overrides map[string]interface{}) (map[string]interface{}, error) { - f, err := os.Open(archivePath) - if err != nil { - return nil, fmt.Errorf("opening bundle %s: %w", archivePath, err) - } - defer f.Close() - - gr, err := gzip.NewReader(f) - if err != nil { - return nil, fmt.Errorf("reading gzip from bundle %s: %w", archivePath, err) - } - defer gr.Close() - - tr := tar.NewReader(gr) - for { - hdr, err := tr.Next() - if errors.Is(err, io.EOF) { - break - } - if err != nil { - return nil, fmt.Errorf("reading tar from bundle %s: %w", archivePath, err) - } - if strings.TrimPrefix(hdr.Name, "./") == "data.json" { - raw, err := io.ReadAll(tr) - if err != nil { - return nil, fmt.Errorf("reading data.json from bundle %s: %w", archivePath, err) - } - var bundleData map[string]interface{} - if err := json.Unmarshal(raw, &bundleData); err != nil { - return nil, fmt.Errorf("parsing data.json from bundle %s: %w", archivePath, err) - } - merged := make(map[string]interface{}, len(bundleData)+len(overrides)) - for k, v := range bundleData { - merged[k] = v - } - for k, v := range overrides { - merged[k] = v - } - return merged, nil - } - } - return overrides, nil -} - // arnCertID extracts the certificate UUID from an ACM ARN. // ARN format: arn:aws:acm:::certificate/ func arnCertID(arn string) string { diff --git a/internal/eval_test.go b/internal/eval_test.go deleted file mode 100644 index 92fb669..0000000 --- a/internal/eval_test.go +++ /dev/null @@ -1,160 +0,0 @@ -package internal - -import ( - "archive/tar" - "compress/gzip" - "os" - "path/filepath" - "testing" -) - -// writeTarGz creates a tar.gz archive at dest containing a single file at -// the given internal path with the given content. -func writeTarGz(t *testing.T, dest, internalPath, content string) { - t.Helper() - f, err := os.Create(dest) - if err != nil { - t.Fatal(err) - } - defer f.Close() - gw := gzip.NewWriter(f) - defer gw.Close() - tw := tar.NewWriter(gw) - defer tw.Close() - body := []byte(content) - if err := tw.WriteHeader(&tar.Header{Name: internalPath, Mode: 0644, Size: int64(len(body))}); err != nil { - t.Fatal(err) - } - if _, err := tw.Write(body); err != nil { - t.Fatal(err) - } -} - -func TestLoadBundleRootData_TarGzBundle(t *testing.T) { - root := t.TempDir() - bundlePath := filepath.Join(root, "bundle.tar.gz") - writeTarGz(t, bundlePath, "data.json", `{"expiry_warning_days":30,"required_certificate_tags":["Environment"]}`) - - result, err := LoadBundleRootData(bundlePath, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if got := result["expiry_warning_days"]; got != float64(30) { - t.Errorf("expiry_warning_days: got %v, want 30", got) - } - if _, ok := result["required_certificate_tags"]; !ok { - t.Error("required_certificate_tags from tar.gz bundle missing from result") - } -} - -func TestLoadBundleRootData_TarGzBundleOverridesWin(t *testing.T) { - root := t.TempDir() - bundlePath := filepath.Join(root, "bundle.tar.gz") - writeTarGz(t, bundlePath, "data.json", `{"expiry_warning_days":30}`) - - overrides := map[string]interface{}{"expiry_warning_days": float64(90)} - result, err := LoadBundleRootData(bundlePath, overrides) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if got := result["expiry_warning_days"]; got != float64(90) { - t.Errorf("expiry_warning_days: got %v, want 90 (operator override must win)", got) - } -} - -func TestLoadBundleRootData_OverridesWinOverBundleDefaults(t *testing.T) { - dir := t.TempDir() - bundleJSON := `{"expiry_warning_days":30,"required_certificate_tags":["Environment"]}` - if err := os.WriteFile(filepath.Join(dir, "data.json"), []byte(bundleJSON), 0644); err != nil { - t.Fatal(err) - } - - overrides := map[string]interface{}{ - "expiry_warning_days": float64(90), - } - - result, err := LoadBundleRootData(dir, overrides) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - // Operator override (90) must win over bundle default (30). - if got := result["expiry_warning_days"]; got != float64(90) { - t.Errorf("expiry_warning_days: got %v, want 90", got) - } - - // Bundle key absent from overrides must still be present. - if _, ok := result["required_certificate_tags"]; !ok { - t.Error("required_certificate_tags from bundle missing from merged result") - } -} - -func TestLoadBundleRootData_NoBundleDataJsonReturnsOverrides(t *testing.T) { - dir := t.TempDir() // no data.json written - - overrides := map[string]interface{}{ - "expiry_warning_days": float64(60), - } - - result, err := LoadBundleRootData(dir, overrides) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if got := result["expiry_warning_days"]; got != float64(60) { - t.Errorf("expiry_warning_days: got %v, want 60", got) - } -} - -func TestLoadBundleRootData_PolicyPathDataJsonWinsOverParent(t *testing.T) { - root := t.TempDir() - parentJSON := `{"expiry_warning_days":30,"source":"parent"}` - if err := os.WriteFile(filepath.Join(root, "data.json"), []byte(parentJSON), 0644); err != nil { - t.Fatal(err) - } - policiesDir := filepath.Join(root, "policies") - if err := os.Mkdir(policiesDir, 0755); err != nil { - t.Fatal(err) - } - policiesJSON := `{"expiry_warning_days":60,"source":"policyPath"}` - if err := os.WriteFile(filepath.Join(policiesDir, "data.json"), []byte(policiesJSON), 0644); err != nil { - t.Fatal(err) - } - - // When both data.json locations exist, policyPath/data.json must win. - result, err := LoadBundleRootData(policiesDir, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if got := result["source"]; got != "policyPath" { - t.Errorf("source: got %v, want policyPath", got) - } - if got := result["expiry_warning_days"]; got != float64(60) { - t.Errorf("expiry_warning_days: got %v, want 60", got) - } -} - -func TestLoadBundleRootData_FindsDataJsonOneDirectoryUp(t *testing.T) { - root := t.TempDir() - bundleJSON := `{"expiry_warning_days":30}` - if err := os.WriteFile(filepath.Join(root, "data.json"), []byte(bundleJSON), 0644); err != nil { - t.Fatal(err) - } - policiesDir := filepath.Join(root, "policies") - if err := os.Mkdir(policiesDir, 0755); err != nil { - t.Fatal(err) - } - - // policyPath is the policies/ subdirectory; data.json is one level up. - result, err := LoadBundleRootData(policiesDir, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if got := result["expiry_warning_days"]; got != float64(30) { - t.Errorf("expiry_warning_days: got %v, want 30", got) - } -} diff --git a/main.go b/main.go index eda4ddb..7842485 100644 --- a/main.go +++ b/main.go @@ -72,23 +72,12 @@ func (l *CompliancePlugin) Eval(request *proto.EvalRequest, apiHelper runner.Api }, fmt.Errorf("failed to fetch data: %w", err) } - // Load bundle data.json defaults and merge with operator overrides once per - // evaluation cycle. All policy paths share the same bundle so one load suffices. - policyData := l.policyData - if paths := request.GetPolicyPaths(); len(paths) > 0 { - merged, err := internal.LoadBundleRootData(paths[0], l.policyData) - if err != nil { - return &proto.EvalResponse{Status: proto.ExecutionStatus_FAILURE}, fmt.Errorf("loading bundle data for %s: %w", paths[0], err) - } - policyData = merged - } - policyEvaluator := internal.NewPolicyEvaluator(ctx, l.logger, activities) var allEvidences []*proto.Evidence var evalErrors error for _, cert := range certs { - certEvidences, err := policyEvaluator.Eval(ctx, cert, request.GetPolicyPaths(), policyData, l.config.PolicyLabels) + certEvidences, err := policyEvaluator.Eval(ctx, cert, request.GetPolicyPaths(), l.policyData, l.config.PolicyLabels) allEvidences = append(allEvidences, certEvidences...) if err != nil { evalErrors = errors.Join(evalErrors, fmt.Errorf("evaluating cert %s: %w", cert.CertificateArn, err))