Skip to content

Commit 3362511

Browse files
leodidoona-agent
andcommitted
fix: fail fast on SPDX normalization errors (P0 fixes)
Critical improvements to SPDX UUID replacement: 1. **Type validation**: Check that documentNamespace is a string - Prevents silent corruption when field has wrong type - Returns clear error message with actual type 2. **Empty validation**: Check that documentNamespace is not empty - Prevents invalid SBOM generation - Fails fast instead of silently continuing 3. **UUID validation**: Fail if no UUID found in namespace - Previously logged warning and continued (non-deterministic!) - Now returns error with helpful message - Alerts to potential Syft format changes 4. **Multiple UUID handling**: Log warning when multiple UUIDs found - Documents intentional behavior (replace all with same UUID) - Helps debugging unexpected formats 5. **Comprehensive edge case tests**: - documentNamespace is not a string - documentNamespace is empty - documentNamespace has no UUID - All cases now properly fail with clear errors Benefits: - Fails fast instead of silently producing non-deterministic builds - Better error messages for debugging - Catches unexpected SBOM format changes - Prevents SBOM corruption Addresses critical issues identified in PR review. Co-authored-by: Ona <no-reply@ona.com>
1 parent 48794b6 commit 3362511

File tree

2 files changed

+70
-33
lines changed

2 files changed

+70
-33
lines changed

pkg/leeway/sbom.go

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -199,9 +199,16 @@ func normalizeSPDX(sbomPath string, timestamp time.Time) error {
199199
}
200200
creationInfo["created"] = timestamp.Format(time.RFC3339)
201201

202+
// Get and validate documentNamespace
203+
originalNamespace, ok := sbom["documentNamespace"].(string)
204+
if !ok {
205+
return fmt.Errorf("documentNamespace field is not a string (got type %T)", sbom["documentNamespace"])
206+
}
207+
if originalNamespace == "" {
208+
return fmt.Errorf("documentNamespace field is empty")
209+
}
210+
202211
// Generate deterministic UUID from normalized content (without timestamp and UUID)
203-
// Save original namespace to extract later
204-
originalNamespace, _ := sbom["documentNamespace"].(string)
205212
delete(sbom, "documentNamespace")
206213

207214
normalizedForHash, err := json.Marshal(sbom)
@@ -212,18 +219,23 @@ func normalizeSPDX(sbomPath string, timestamp time.Time) error {
212219
deterministicUUID := generateDeterministicUUID(contentHash[:])
213220

214221
// Replace UUID in documentNamespace using regex for robust matching
215-
if originalNamespace != "" {
216-
// UUID pattern: 8-4-4-4-12 hex digits
217-
uuidPattern := regexp.MustCompile(`[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}`)
218-
if uuidPattern.MatchString(originalNamespace) {
219-
// Replace the UUID with our deterministic one
220-
originalNamespace = uuidPattern.ReplaceAllString(originalNamespace, deterministicUUID)
221-
} else {
222-
// Log warning if no UUID found in namespace (unexpected format)
223-
log.WithField("documentNamespace", originalNamespace).Warn("No UUID found in SPDX documentNamespace, skipping UUID normalization")
224-
}
225-
sbom["documentNamespace"] = originalNamespace
222+
// UUID pattern: 8-4-4-4-12 hex digits
223+
uuidPattern := regexp.MustCompile(`[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}`)
224+
225+
matches := uuidPattern.FindAllString(originalNamespace, -1)
226+
if len(matches) == 0 {
227+
return fmt.Errorf("no UUID found in SPDX documentNamespace: %s. "+
228+
"This may indicate a format change in Syft. Please report this issue", originalNamespace)
226229
}
230+
if len(matches) > 1 {
231+
log.WithField("documentNamespace", originalNamespace).
232+
WithField("uuid_count", len(matches)).
233+
Warn("Multiple UUIDs found in documentNamespace, replacing all with same deterministic UUID")
234+
}
235+
236+
// Replace the UUID(s) with our deterministic one
237+
originalNamespace = uuidPattern.ReplaceAllString(originalNamespace, deterministicUUID)
238+
sbom["documentNamespace"] = originalNamespace
227239

228240
// Write back
229241
normalized, err := json.MarshalIndent(sbom, "", " ")

pkg/leeway/sbom_normalize_test.go

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -266,26 +266,25 @@ func TestNormalizeSPDX(t *testing.T) {
266266
name string
267267
documentNamespace string
268268
wantUUIDChanged bool
269+
wantMultipleWarn bool
269270
}{
270271
{
271272
name: "UUID at end of namespace",
272273
documentNamespace: "https://example.com/test-12345678-1234-1234-1234-123456789abc",
273274
wantUUIDChanged: true,
275+
wantMultipleWarn: false,
274276
},
275277
{
276278
name: "UUID in middle of namespace",
277279
documentNamespace: "https://example.com/12345678-1234-1234-1234-123456789abc/test",
278280
wantUUIDChanged: true,
281+
wantMultipleWarn: false,
279282
},
280283
{
281284
name: "multiple UUIDs (replaces all)",
282285
documentNamespace: "https://example.com/12345678-1234-1234-1234-123456789abc/test-abcdef01-2345-6789-abcd-ef0123456789",
283286
wantUUIDChanged: true,
284-
},
285-
{
286-
name: "no UUID in namespace",
287-
documentNamespace: "https://example.com/test-no-uuid",
288-
wantUUIDChanged: false,
287+
wantMultipleWarn: true,
289288
},
290289
}
291290

@@ -353,20 +352,13 @@ func TestNormalizeSPDX(t *testing.T) {
353352

354353
// Verify UUID in documentNamespace
355354
namespace := normalizedSBOM["documentNamespace"].(string)
356-
if tt.wantUUIDChanged {
357-
// Should have changed from original
358-
if namespace == tt.documentNamespace {
359-
t.Error("UUID in documentNamespace was not changed")
360-
}
361-
// Should not contain the original UUID
362-
if contains(namespace, "12345678-1234-1234-1234-123456789abc") {
363-
t.Error("original UUID still present in documentNamespace")
364-
}
365-
} else {
366-
// Should remain unchanged if no UUID was present
367-
if namespace != tt.documentNamespace {
368-
t.Errorf("documentNamespace changed unexpectedly: got %s, want %s", namespace, tt.documentNamespace)
369-
}
355+
// Should have changed from original
356+
if namespace == tt.documentNamespace {
357+
t.Error("UUID in documentNamespace was not changed")
358+
}
359+
// Should not contain the original UUID
360+
if contains(namespace, "12345678-1234-1234-1234-123456789abc") {
361+
t.Error("original UUID still present in documentNamespace")
370362
}
371363

372364
// Normalize again with same timestamp - should produce same result
@@ -463,7 +455,7 @@ func TestNormalizeSPDX_MalformedSBOM(t *testing.T) {
463455
},
464456
{
465457
name: "missing creationInfo field",
466-
sbomContent: `{"spdxVersion": "SPDX-2.3", "name": "test"}`,
458+
sbomContent: `{"spdxVersion": "SPDX-2.3", "name": "test", "documentNamespace": "https://example.com/test-12345678-1234-1234-1234-123456789abc"}`,
467459
wantErr: true,
468460
errContains: "creationInfo field not found",
469461
},
@@ -473,6 +465,39 @@ func TestNormalizeSPDX_MalformedSBOM(t *testing.T) {
473465
wantErr: true,
474466
errContains: "failed to parse SBOM",
475467
},
468+
{
469+
name: "documentNamespace is not a string",
470+
sbomContent: `{
471+
"spdxVersion": "SPDX-2.3",
472+
"name": "test",
473+
"documentNamespace": 12345,
474+
"creationInfo": {"created": "2023-01-01T00:00:00Z"}
475+
}`,
476+
wantErr: true,
477+
errContains: "documentNamespace field is not a string",
478+
},
479+
{
480+
name: "documentNamespace is empty",
481+
sbomContent: `{
482+
"spdxVersion": "SPDX-2.3",
483+
"name": "test",
484+
"documentNamespace": "",
485+
"creationInfo": {"created": "2023-01-01T00:00:00Z"}
486+
}`,
487+
wantErr: true,
488+
errContains: "documentNamespace field is empty",
489+
},
490+
{
491+
name: "documentNamespace has no UUID",
492+
sbomContent: `{
493+
"spdxVersion": "SPDX-2.3",
494+
"name": "test",
495+
"documentNamespace": "https://example.com/no-uuid-here",
496+
"creationInfo": {"created": "2023-01-01T00:00:00Z"}
497+
}`,
498+
wantErr: true,
499+
errContains: "no UUID found in SPDX documentNamespace",
500+
},
476501
}
477502

478503
for _, tt := range tests {

0 commit comments

Comments
 (0)