Skip to content

Commit 48794b6

Browse files
leodidoona-agent
andcommitted
refactor: use regex for robust UUID replacement in SPDX namespace
Replace manual string manipulation with regex-based UUID matching: - Use regex pattern to find and replace UUIDs - Validate that a UUID exists before attempting replacement - Handle edge cases: UUID at end, in middle, multiple UUIDs, no UUID - Log warning if no UUID found in namespace (unexpected format) - Add comprehensive test coverage for all edge cases Benefits: - More robust: validates UUID format before replacement - Handles any UUID position in the namespace - Fails gracefully with warning instead of silently - Better test coverage for edge cases Addresses review feedback from Cornelius on PR #281. Co-authored-by: Ona <no-reply@ona.com>
1 parent 9cf830c commit 48794b6

File tree

2 files changed

+124
-86
lines changed

2 files changed

+124
-86
lines changed

pkg/leeway/sbom.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"os"
1313
"os/exec"
1414
"path/filepath"
15+
"regexp"
1516
"runtime"
1617
"strconv"
1718
"strings"
@@ -210,22 +211,16 @@ func normalizeSPDX(sbomPath string, timestamp time.Time) error {
210211
contentHash := sha256.Sum256(normalizedForHash)
211212
deterministicUUID := generateDeterministicUUID(contentHash[:])
212213

213-
// Replace UUID in documentNamespace
214+
// Replace UUID in documentNamespace using regex for robust matching
214215
if originalNamespace != "" {
215-
// Extract base URL and replace UUID at the end
216-
lastDash := strings.LastIndex(originalNamespace, "-")
217-
if lastDash > 0 {
218-
// Find the start of the UUID (5 segments separated by dashes)
219-
uuidStart := lastDash
220-
for i := 0; i < 4; i++ {
221-
uuidStart = strings.LastIndex(originalNamespace[:uuidStart], "-")
222-
if uuidStart == -1 {
223-
break
224-
}
225-
}
226-
if uuidStart > 0 {
227-
originalNamespace = originalNamespace[:uuidStart] + "-" + deterministicUUID
228-
}
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")
229224
}
230225
sbom["documentNamespace"] = originalNamespace
231226
}

pkg/leeway/sbom_normalize_test.go

Lines changed: 114 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -262,90 +262,133 @@ func TestNormalizeCycloneDX(t *testing.T) {
262262
}
263263

264264
func TestNormalizeSPDX(t *testing.T) {
265-
// Create a temporary directory for test files
266-
tmpDir := t.TempDir()
267-
sbomPath := filepath.Join(tmpDir, "sbom.spdx.json")
268-
269-
// Create a sample SPDX SBOM
270-
sbom := map[string]interface{}{
271-
"spdxVersion": "SPDX-2.3",
272-
"dataLicense": "CC0-1.0",
273-
"SPDXID": "SPDXRef-DOCUMENT",
274-
"name": "test-sbom",
275-
"documentNamespace": "https://example.com/test-12345678-1234-1234-1234-123456789abc",
276-
"creationInfo": map[string]interface{}{
277-
"created": "2023-01-01T00:00:00Z",
278-
"creators": []interface{}{
279-
"Tool: test-tool",
280-
},
265+
tests := []struct {
266+
name string
267+
documentNamespace string
268+
wantUUIDChanged bool
269+
}{
270+
{
271+
name: "UUID at end of namespace",
272+
documentNamespace: "https://example.com/test-12345678-1234-1234-1234-123456789abc",
273+
wantUUIDChanged: true,
281274
},
282-
"packages": []interface{}{
283-
map[string]interface{}{
284-
"SPDXID": "SPDXRef-Package",
285-
"name": "test-package",
286-
"version": "1.0.0",
287-
},
275+
{
276+
name: "UUID in middle of namespace",
277+
documentNamespace: "https://example.com/12345678-1234-1234-1234-123456789abc/test",
278+
wantUUIDChanged: true,
279+
},
280+
{
281+
name: "multiple UUIDs (replaces all)",
282+
documentNamespace: "https://example.com/12345678-1234-1234-1234-123456789abc/test-abcdef01-2345-6789-abcd-ef0123456789",
283+
wantUUIDChanged: true,
284+
},
285+
{
286+
name: "no UUID in namespace",
287+
documentNamespace: "https://example.com/test-no-uuid",
288+
wantUUIDChanged: false,
288289
},
289290
}
290291

291-
// Write initial SBOM
292-
data, err := json.MarshalIndent(sbom, "", " ")
293-
if err != nil {
294-
t.Fatalf("failed to marshal test SBOM: %v", err)
295-
}
296-
if err := os.WriteFile(sbomPath, data, 0644); err != nil {
297-
t.Fatalf("failed to write test SBOM: %v", err)
298-
}
292+
for _, tt := range tests {
293+
t.Run(tt.name, func(t *testing.T) {
294+
// Create a temporary directory for test files
295+
tmpDir := t.TempDir()
296+
sbomPath := filepath.Join(tmpDir, "sbom.spdx.json")
297+
298+
// Create a sample SPDX SBOM
299+
sbom := map[string]interface{}{
300+
"spdxVersion": "SPDX-2.3",
301+
"dataLicense": "CC0-1.0",
302+
"SPDXID": "SPDXRef-DOCUMENT",
303+
"name": "test-sbom",
304+
"documentNamespace": tt.documentNamespace,
305+
"creationInfo": map[string]interface{}{
306+
"created": "2023-01-01T00:00:00Z",
307+
"creators": []interface{}{
308+
"Tool: test-tool",
309+
},
310+
},
311+
"packages": []interface{}{
312+
map[string]interface{}{
313+
"SPDXID": "SPDXRef-Package",
314+
"name": "test-package",
315+
"version": "1.0.0",
316+
},
317+
},
318+
}
299319

300-
// Normalize with a fixed timestamp
301-
fixedTime := time.Unix(1234567890, 0).UTC()
302-
if err := normalizeSPDX(sbomPath, fixedTime); err != nil {
303-
t.Fatalf("normalizeSPDX failed: %v", err)
304-
}
320+
// Write initial SBOM
321+
data, err := json.MarshalIndent(sbom, "", " ")
322+
if err != nil {
323+
t.Fatalf("failed to marshal test SBOM: %v", err)
324+
}
325+
if err := os.WriteFile(sbomPath, data, 0644); err != nil {
326+
t.Fatalf("failed to write test SBOM: %v", err)
327+
}
305328

306-
// Read normalized SBOM
307-
normalizedData, err := os.ReadFile(sbomPath)
308-
if err != nil {
309-
t.Fatalf("failed to read normalized SBOM: %v", err)
310-
}
329+
// Normalize with a fixed timestamp
330+
fixedTime := time.Unix(1234567890, 0).UTC()
331+
if err := normalizeSPDX(sbomPath, fixedTime); err != nil {
332+
t.Fatalf("normalizeSPDX failed: %v", err)
333+
}
311334

312-
var normalizedSBOM map[string]interface{}
313-
if err := json.Unmarshal(normalizedData, &normalizedSBOM); err != nil {
314-
t.Fatalf("failed to parse normalized SBOM: %v", err)
315-
}
335+
// Read normalized SBOM
336+
normalizedData, err := os.ReadFile(sbomPath)
337+
if err != nil {
338+
t.Fatalf("failed to read normalized SBOM: %v", err)
339+
}
316340

317-
// Verify timestamp was updated
318-
creationInfo := normalizedSBOM["creationInfo"].(map[string]interface{})
319-
created := creationInfo["created"].(string)
320-
expectedTimestamp := fixedTime.Format(time.RFC3339)
321-
if created != expectedTimestamp {
322-
t.Errorf("expected timestamp %s, got %s", expectedTimestamp, created)
323-
}
341+
var normalizedSBOM map[string]interface{}
342+
if err := json.Unmarshal(normalizedData, &normalizedSBOM); err != nil {
343+
t.Fatalf("failed to parse normalized SBOM: %v", err)
344+
}
324345

325-
// Verify UUID in documentNamespace was changed
326-
namespace := normalizedSBOM["documentNamespace"].(string)
327-
if namespace == "https://example.com/test-12345678-1234-1234-1234-123456789abc" {
328-
t.Error("UUID in documentNamespace was not changed")
329-
}
346+
// Verify timestamp was updated
347+
creationInfo := normalizedSBOM["creationInfo"].(map[string]interface{})
348+
created := creationInfo["created"].(string)
349+
expectedTimestamp := fixedTime.Format(time.RFC3339)
350+
if created != expectedTimestamp {
351+
t.Errorf("expected timestamp %s, got %s", expectedTimestamp, created)
352+
}
330353

331-
// Normalize again with same timestamp - should produce same UUID
332-
if err := normalizeSPDX(sbomPath, fixedTime); err != nil {
333-
t.Fatalf("second normalizeSPDX failed: %v", err)
334-
}
354+
// Verify UUID in documentNamespace
355+
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+
}
370+
}
335371

336-
normalizedData2, err := os.ReadFile(sbomPath)
337-
if err != nil {
338-
t.Fatalf("failed to read normalized SBOM (2nd time): %v", err)
339-
}
372+
// Normalize again with same timestamp - should produce same result
373+
if err := normalizeSPDX(sbomPath, fixedTime); err != nil {
374+
t.Fatalf("second normalizeSPDX failed: %v", err)
375+
}
340376

341-
var normalizedSBOM2 map[string]interface{}
342-
if err := json.Unmarshal(normalizedData2, &normalizedSBOM2); err != nil {
343-
t.Fatalf("failed to parse normalized SBOM (2nd time): %v", err)
344-
}
377+
normalizedData2, err := os.ReadFile(sbomPath)
378+
if err != nil {
379+
t.Fatalf("failed to read normalized SBOM (2nd time): %v", err)
380+
}
345381

346-
namespace2 := normalizedSBOM2["documentNamespace"].(string)
347-
if namespace != namespace2 {
348-
t.Errorf("expected deterministic UUID, got %s and %s", namespace, namespace2)
382+
var normalizedSBOM2 map[string]interface{}
383+
if err := json.Unmarshal(normalizedData2, &normalizedSBOM2); err != nil {
384+
t.Fatalf("failed to parse normalized SBOM (2nd time): %v", err)
385+
}
386+
387+
namespace2 := normalizedSBOM2["documentNamespace"].(string)
388+
if namespace != namespace2 {
389+
t.Errorf("expected deterministic result, got %s and %s", namespace, namespace2)
390+
}
391+
})
349392
}
350393
}
351394

0 commit comments

Comments
 (0)