From 42c8dabdf2cad54087a112ba2e1bba1535a5038d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20Garc=C3=ADa=20Veytia=20=28Puerco=29?= Date: Mon, 14 Sep 2026 17:57:00 -0600 Subject: [PATCH] Parse policy materials written in HJSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The policy, policy set and policy group parsers fed raw bytes to protojson, so policies authored in HJSON (comments, unquoted keys, multiline strings) failed as unparseable when read through the attestation collector, even though the policy module itself accepts both formats. Normalize input to JSON before unmarshaling: valid JSON is untouched, HJSON is converted, and anything else is reported as not this format so other predicate parsers get their turn. On the HJSON path an empty or non-object document is rejected, since HJSON's braceless root would otherwise turn empty input into an empty policy. Signed-off-by: Adolfo García Veytia (Puerco) --- go.mod | 1 + go.sum | 2 + hjson.go | 56 ++++++++++ hjson_test.go | 202 +++++++++++++++++++++++++++++++++++ parser.go | 12 +++ testdata/test-policy.json | 22 ++++ testdata/test-policyset.json | 31 ++++++ testdata/test-results.json | 104 ++++++++++++++++++ 8 files changed, 430 insertions(+) create mode 100644 hjson.go create mode 100644 hjson_test.go create mode 100644 testdata/test-policy.json create mode 100644 testdata/test-policyset.json create mode 100644 testdata/test-results.json diff --git a/go.mod b/go.mod index 55b5c42..a9d9eca 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.26.0 require ( github.com/carabiner-dev/attestation v0.2.1 github.com/carabiner-dev/policy v0.5.5 + github.com/hjson/hjson-go/v4 v4.7.1 google.golang.org/protobuf v1.36.12 ) diff --git a/go.sum b/go.sum index 6a5734b..653bbc7 100644 --- a/go.sum +++ b/go.sum @@ -231,6 +231,8 @@ github.com/hashicorp/hcl v1.0.1-vault-7 h1:ag5OxFVy3QYTFTJODRzTKVZ6xvdfLLCA1cy/Y github.com/hashicorp/hcl v1.0.1-vault-7/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM= github.com/hashicorp/vault/api v1.23.0 h1:gXgluBsSECfRWTSW9niY2jwg2e9mMJc4WoHNv4g3h6A= github.com/hashicorp/vault/api v1.23.0/go.mod h1:zransKiB9ftp+kgY8ydjnvCU7Wk8i9L0DYWpXeMj9ko= +github.com/hjson/hjson-go/v4 v4.7.1 h1:nC/dZ7GCvcFa9KXR3YJzufloeWRLovFAI4XmiAl7jy8= +github.com/hjson/hjson-go/v4 v4.7.1/go.mod h1:4zx6c7Y0vWcm8IRyVoQJUHAPJLXLvbG6X8nk1RLigSo= github.com/howeyc/gopass v0.0.0-20210920133722-c8aef6fb66ef h1:A9HsByNhogrvm9cWb28sjiS3i7tcKCkflWFEkHfuAgM= github.com/howeyc/gopass v0.0.0-20210920133722-c8aef6fb66ef/go.mod h1:lADxMC39cJJqL93Duh1xhAs4I2Zs8mKS89XWXFGp9cs= github.com/in-toto/attestation v1.2.0 h1:aPRUZ3azbqD7yEBD5fP3TD8Dszf+YHo284SOcpahjQk= diff --git a/hjson.go b/hjson.go new file mode 100644 index 0000000..4864c95 --- /dev/null +++ b/hjson.go @@ -0,0 +1,56 @@ +// SPDX-FileCopyrightText: Copyright 2025 Carabiner Systems, Inc +// SPDX-License-Identifier: Apache-2.0 + +package predicates + +import ( + "bytes" + "encoding/json" + + "github.com/carabiner-dev/attestation" + "github.com/hjson/hjson-go/v4" +) + +// normalizeToJSON returns the JSON form of a policy material document. +// Policies, policy sets and groups are authored in JSON or HJSON (comments, +// unquoted keys, multiline strings); attestations only ever carry JSON. Valid +// JSON is returned untouched, HJSON is converted, and data that is neither +// (or that does not describe an object) is reported as not being a policy +// document at all through attestation.ErrNotCorrectFormat, so the caller +// can move on to other predicate parsers. +func normalizeToJSON(data []byte) ([]byte, error) { + // HJSON accepts a braceless root object, so empty input would parse + // as an empty document. Reject it explicitly. + if len(bytes.TrimSpace(data)) == 0 { + return nil, attestation.ErrNotCorrectFormat + } + if json.Valid(data) { + if !isJSONObject(data) { + return nil, attestation.ErrNotCorrectFormat + } + return data, nil + } + + // HJSON is lenient (braceless roots, bare words as strings), so noise + // can parse as an empty object. Only a non-empty object counts as a + // human-authored policy document. + var parsed any + if err := hjson.Unmarshal(data, &parsed); err != nil { + return nil, attestation.ErrNotCorrectFormat + } + doc, ok := parsed.(map[string]any) + if !ok || len(doc) == 0 { + return nil, attestation.ErrNotCorrectFormat + } + normalized, err := json.Marshal(parsed) + if err != nil { + return nil, attestation.ErrNotCorrectFormat + } + return normalized, nil +} + +// isJSONObject reports if a valid JSON document is an object. +func isJSONObject(data []byte) bool { + trimmed := bytes.TrimLeft(data, " \t\r\n") + return len(trimmed) > 0 && trimmed[0] == '{' +} diff --git a/hjson_test.go b/hjson_test.go new file mode 100644 index 0000000..de06809 --- /dev/null +++ b/hjson_test.go @@ -0,0 +1,202 @@ +// SPDX-FileCopyrightText: Copyright 2025 Carabiner Systems, Inc +// SPDX-License-Identifier: Apache-2.0 + +package predicates + +import ( + "encoding/json" + "errors" + "testing" + + "github.com/carabiner-dev/attestation" + "google.golang.org/protobuf/encoding/protojson" + + papi "github.com/carabiner-dev/policy/api/v1" +) + +// hjsonPolicySet is a policy set as humans write it: comments, unquoted +// keys, a multiline description and trailing commas. +const hjsonPolicySet = `{ + // A policy set written in HJSON + id: hjson-set + meta: { + description: + ''' + # Release verification + Multi-line markdown. + ''' + version: 1, + } + common: { + identities: [ + { + sigstore: { + issuerMatch: { exact: "https://token.actions.githubusercontent.com" } + identityMatch: { regex: "^https://github\\.com/org/repo/\\.github/workflows/release\\.yaml@refs/tags/v.*$" } + } + } + ] + } + policies: [ + { + id: slsa-builder-id + source: { location: { uri: "git+https://github.com/carabiner-dev/policies#slsa/slsa-builder-id.json" } } + }, + ] +}` + +const hjsonPolicy = `{ + id: has-provenance + meta: { description: "Requires a provenance attestation" } + tenets: [ + { + id: exists + predicates: { types: ["https://slsa.dev/provenance/v1"] } + code: "size(predicates) > 0" + } + ] +}` + +const hjsonPolicyGroup = `{ + id: release-group + meta: { description: "A group" } + blocks: [ + { + id: block-a + policies: [ { id: "has-provenance" } ] + } + ] +}` + +func TestNormalizeToJSON(t *testing.T) { + for _, tc := range []struct { + name string + data string + wantNotFormat bool + wantID any + }{ + {"json-untouched", `{"id": "x", "meta": {"version": 1}}`, false, "x"}, + {"hjson-converted", "{\n // comment\n id: y\n meta: { version: 1 }\n}", false, "y"}, + {"hjson-braceless-root", "id: z\nmeta: { version: 1 }", false, "z"}, + {"empty", "", true, ""}, + {"whitespace", " \n\t", true, ""}, + {"json-array", `[1, 2]`, true, ""}, + {"json-string", `"just a string"`, true, ""}, + {"binary-garbage", "\x00\x01{{{", true, ""}, + {"hjson-empty-document", "// nothing here\n", true, ""}, + {"json-empty-object-kept", `{}`, false, nil}, + {"unbalanced", "{ id: x", true, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + out, err := normalizeToJSON([]byte(tc.data)) + if tc.wantNotFormat { + if !errors.Is(err, attestation.ErrNotCorrectFormat) { + t.Fatalf("expected ErrNotCorrectFormat, got err=%v out=%q", err, out) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !json.Valid(out) { + t.Fatalf("normalized output is not JSON: %q", out) + } + var doc map[string]any + if err := json.Unmarshal(out, &doc); err != nil { + t.Fatal(err) + } + if doc["id"] != tc.wantID { + t.Fatalf("id = %v, want %v", doc["id"], tc.wantID) + } + }) + } +} + +func TestParser_HJSONPolicyMaterials(t *testing.T) { + parser := New() + + t.Run("policyset", func(t *testing.T) { + pred, err := parser.ParsePolicySetPredicate([]byte(hjsonPolicySet)) + if err != nil { + t.Fatalf("ParsePolicySetPredicate() error = %v", err) + } + set, ok := pred.(*PolicySet) + if !ok { + t.Fatalf("wrong type %T", pred) + } + if set.Parsed.GetId() != "hjson-set" { + t.Errorf("id = %q", set.Parsed.GetId()) + } + if set.Parsed.GetMeta().GetVersion() != 1 { + t.Errorf("version = %d", set.Parsed.GetMeta().GetVersion()) + } + if got := set.Parsed.GetCommon().GetIdentities()[0].GetSigstore().GetIdentityMatch().GetRegex(); got == "" { + t.Error("identity regex was lost in normalization") + } + if got := set.Parsed.GetPolicies()[0].GetSource().GetLocation().GetUri(); got == "" { + t.Error("remote reference was lost in normalization") + } + // The data exposed downstream is JSON, so consumers that only speak + // JSON (protojson, the policy parser) can read it back. + if !json.Valid(set.GetData()) { + t.Fatalf("GetData() is not JSON: %q", set.GetData()) + } + reparsed := &papi.PolicySet{} + if err := protojson.Unmarshal(set.GetData(), reparsed); err != nil { + t.Fatalf("GetData() does not round-trip through protojson: %v", err) + } + if set.GetType() != PredicateTypePolicySet { + t.Errorf("type = %s", set.GetType()) + } + }) + + t.Run("policy", func(t *testing.T) { + pred, err := parser.ParsePolicyPredicate([]byte(hjsonPolicy)) + if err != nil { + t.Fatalf("ParsePolicyPredicate() error = %v", err) + } + policy, ok := pred.(*Policy) + if !ok { + t.Fatalf("wrong type %T", pred) + } + if policy.Parsed.GetId() != "has-provenance" || len(policy.Parsed.GetTenets()) != 1 { + t.Errorf("unexpected policy: %v", policy.Parsed) + } + if !json.Valid(policy.GetData()) { + t.Error("GetData() is not JSON") + } + }) + + t.Run("policygroup", func(t *testing.T) { + pred, err := parser.ParsePolicyGroupPredicate([]byte(hjsonPolicyGroup)) + if err != nil { + t.Fatalf("ParsePolicyGroupPredicate() error = %v", err) + } + group, ok := pred.(*PolicyGroup) + if !ok { + t.Fatalf("wrong type %T", pred) + } + if group.Parsed.GetId() != "release-group" || len(group.Parsed.GetBlocks()) != 1 { + t.Errorf("unexpected group: %v", group.Parsed) + } + }) + + t.Run("dispatcher-detects-hjson-policyset", func(t *testing.T) { + pred, err := parser.Parse([]byte(hjsonPolicySet)) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if pred.GetType() != PredicateTypePolicySet { + t.Errorf("type = %s, want %s", pred.GetType(), PredicateTypePolicySet) + } + }) + + t.Run("not-a-document-is-not-this-format", func(t *testing.T) { + for _, data := range []string{"", " ", "[1,2]", "\"text\""} { + _, err := parser.ParsePolicySetPredicate([]byte(data)) + if !errors.Is(err, attestation.ErrNotCorrectFormat) { + t.Errorf("data %q: expected ErrNotCorrectFormat, got %v", data, err) + } + } + }) +} diff --git a/parser.go b/parser.go index 921a54b..eef05c4 100644 --- a/parser.go +++ b/parser.go @@ -106,6 +106,10 @@ func (p *ParserPolicySetPredicate) SupportsType(predTypes ...attestation.Predica } func (p *Parser) ParsePolicySetPredicate(data []byte) (attestation.Predicate, error) { + data, err := normalizeToJSON(data) + if err != nil { + return nil, err + } set := &papi.PolicySet{} if err := protojson.Unmarshal(data, set); err != nil { if strings.Contains(err.Error(), "proto:") && strings.Contains(err.Error(), "unknown field") { @@ -139,6 +143,10 @@ func (p *ParserPolicyPredicate) SupportsType(predTypes ...attestation.PredicateT } func (p *Parser) ParsePolicyPredicate(data []byte) (attestation.Predicate, error) { + data, err := normalizeToJSON(data) + if err != nil { + return nil, err + } policy := &papi.Policy{} if err := protojson.Unmarshal(data, policy); err != nil { if strings.Contains(err.Error(), "proto:") && strings.Contains(err.Error(), "unknown field") { @@ -169,6 +177,10 @@ func (p *ParserPolicyGroupPredicate) SupportsType(predTypes ...attestation.Predi } func (p *Parser) ParsePolicyGroupPredicate(data []byte) (attestation.Predicate, error) { + data, err := normalizeToJSON(data) + if err != nil { + return nil, err + } group := &papi.PolicyGroup{} if err := protojson.Unmarshal(data, group); err != nil { if strings.Contains(err.Error(), "proto:") && strings.Contains(err.Error(), "unknown field") { diff --git a/testdata/test-policy.json b/testdata/test-policy.json new file mode 100644 index 0000000..123bd4d --- /dev/null +++ b/testdata/test-policy.json @@ -0,0 +1,22 @@ +{ + "id": "has-provenance", + "meta": { + "description": "Verify the artifact was built in the expected environment", + "assert_mode": "AND" + }, + "tenets": [ + { + "code": "size(predicates) > 0", + "predicates": { + "types": ["https://slsa.dev/provenance/v0.2"] + }, + "assessment": { + "message": "Found a signed SLSA provenance attestation" + }, + "error": { + "message": "No provenance data found", + "guidance": "Set up the project's build to generate a slsa attestation" + } + } + ] +} diff --git a/testdata/test-policyset.json b/testdata/test-policyset.json new file mode 100644 index 0000000..f5d87ba --- /dev/null +++ b/testdata/test-policyset.json @@ -0,0 +1,31 @@ +{ + "id": "slsa", + + "meta": { + "runtime": "cel@v0" + }, + "policies": [ + { + "id": "has-provenance", + "meta": { + "description": "Verify the artifact was built in the expected environment", + "assert_mode": "AND" + }, + "tenets": [ + { + "code": "size(predicates) > 0", + "predicates": { + "types": ["https://slsa.dev/provenance/v0.2"] + }, + "assessment": { + "message": "Found a signed SLSA provenance attestation" + }, + "error": { + "message": "No provenance data found", + "guidance": "Set up the project's build to generate a slsa attestation" + } + } + ] + } + ] +} diff --git a/testdata/test-results.json b/testdata/test-results.json new file mode 100644 index 0000000..e4958b8 --- /dev/null +++ b/testdata/test-results.json @@ -0,0 +1,104 @@ +{ + "date_start": "2025-05-06T01:11:25.904Z", + "date_end": "2025-05-06T01:11:25.904Z", + "results": [ + { + "date_start": "2025-05-06T01:11:25.904Z", + "date_end": "2025-05-06T01:11:25.905Z", + "status": "PASS", + "policy": { + "id": "OSPS-GV-02" + }, + "eval_results": [ + { + "date": "2025-05-06T01:11:25.904Z", + "status": "PASS", + "output": {}, + "statements": [ + { + "type": "http://github.com/carabiner-dev/snappy/specs/repo.yaml", + "attestation": { + "digest": { + "sha256": "c89eb6a4518042b28365fdd42d86575fd2030499d6f2e9295b32b1d9cf896da5", + "sha512": "8cd0a3ee3d2e041281a2b01034f36a21f5d548a22f294e3adb19caa238b22d853fe73701b8c3dbfcb17f146e3c47f49109ec51acc6da075981813eb8b4555a51" + } + } + } + ], + "assessment": { + "message": "Found attested repository data" + } + }, + { + "date": "2025-05-06T01:11:25.905Z", + "id": "01", + "status": "PASS", + "output": { + "issues": true + }, + "statements": [ + { + "type": "http://github.com/carabiner-dev/snappy/specs/repo.yaml", + "attestation": { + "digest": { + "sha256": "c89eb6a4518042b28365fdd42d86575fd2030499d6f2e9295b32b1d9cf896da5", + "sha512": "8cd0a3ee3d2e041281a2b01034f36a21f5d548a22f294e3adb19caa238b22d853fe73701b8c3dbfcb17f146e3c47f49109ec51acc6da075981813eb8b4555a51" + } + } + } + ], + "assessment": { + "message": "Issues feature is enabled in the repository" + } + } + ], + "meta": { + "description": "The project MUST have one or more mechanisms for public discussions about proposed changes and usage obstacles.", + "assert_mode": "AND", + "controls": [ + { + "class": "OSPS", + "id": "GV-02" + } + ], + "enforce": "ON" + }, + "chain": [ + { + "source": { + "name": "hello-world-linux-amd64", + "uri": "hello-world-linux-amd64", + "digest": { + "sha256": "2d64e0b39fbe4ca33c8f2365eb1646e2cfb66d69c7e1f94ad95ab89e1e78be69", + "sha512": "3f4116a21a3f85a9856109366ce543e0922dd7cec75741f186a34b1cff1c255adafcd7899c9f6d01d6a0458f3512c45e07c845faffde7cb5908341cd443ce134" + } + }, + "destination": { + "name": "github.com/carabiner-dev/demo-repo", + "uri": "https://github.com/carabiner-dev/demo-repo", + "digest": { + "sha256": "73b9f9806668f6fccf8f49cb0c778bb4fb054f49003ba726fd59913e429154da" + } + }, + "link": { + "type": "https://slsa.dev/provenance/v0.2", + "attestation": { + "digest": { + "sha256": "082b4d976f26b5e49007ca4a6f627963e06dd887ec088d92b491c34414b400cb", + "sha512": "7533402fd875440db27cf2c95fde1411a86057bf774ff684bd047f98913bf050a8002c20aef47b80d4a1790d8b4785838000f71ebb2560d6374700e562fd017d" + } + } + } + } + ], + "subject": { + "name": "github.com/carabiner-dev/demo-repo", + "uri": "https://github.com/carabiner-dev/demo-repo", + "digest": { + "sha256": "73b9f9806668f6fccf8f49cb0c778bb4fb054f49003ba726fd59913e429154da" + } + } + } + ] + +}