Skip to content

Commit f992ccb

Browse files
committed
feat(cli): limit archive explode to an SBOM/SARIF allowlist
Address review feedback: explode only kinds with a meaningful bundle form (SBOM_CYCLONEDX_JSON, SBOM_SPDX_JSON, SARIF) instead of exploding any archive that is not archive-native. Every other kind (ARTIFACT, EVIDENCE, ZAP_DAST_ZIP, …) records the archive whole, so a regular zip provided as a single material is never expanded. Replaces the archive-native denylist with an explodable-kind allowlist. Also consolidate name sanitization: SanitizeMaterialName now returns the bare sanitized component (empty when nothing remains) and callers supply their fallback, so the action layer's sanitizeMaterialNamePart reuses it instead of duplicating the logic. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Chainloop-Trace-Sessions: da72e107-14e9-4da1-add0-28004f542628, ef6d3cdb-5a23-445c-b39a-510b659023e4
1 parent 123f890 commit f992ccb

4 files changed

Lines changed: 56 additions & 55 deletions

File tree

‎app/cli/pkg/action/attestation_add.go‎

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -225,10 +225,14 @@ func (action *AttestationAdd) Run(ctx context.Context, attestationID, materialNa
225225
}
226226

227227
// shouldExplode decides whether an att-add should explode the value into many
228-
// materials: only when --kind is set, the value is a supported archive, and the
229-
// kind is not archive-native (e.g. ZAP_DAST_ZIP, which is recorded whole).
228+
// materials: only when the kind is explodable (SBOM/SARIF) and the value is a
229+
// supported archive. It returns ArchiveNone for every other kind so a regular
230+
// zip provided as e.g. ARTIFACT or EVIDENCE is recorded whole.
230231
func shouldExplode(materialType, value string) (materials.ArchiveFormat, error) {
231-
if materialType == "" || materials.IsArchiveNativeKind(materialType) {
232+
// Only explode kinds that have a meaningful "bundle of the same kind"
233+
// archive form (SBOM, SARIF). Any other kind — including ARTIFACT and
234+
// EVIDENCE — records the archive whole even when the value is a zip/tar.
235+
if !materials.IsExplodableKind(materialType) {
232236
return materials.ArchiveNone, nil
233237
}
234238
return materials.DetectArchive(value)
@@ -370,29 +374,14 @@ func policyInputEvidenceNames(materialName string, policyInputFiles []*PolicyInp
370374
return names
371375
}
372376

373-
// sanitizeMaterialNamePart lower-cases s and collapses every run of characters
374-
// outside [a-z0-9] into a single "-", trimming leading/trailing "-", so the
375-
// result is a valid material-name component. Falls back to "input" if nothing
376-
// usable remains.
377+
// sanitizeMaterialNamePart sanitizes s into a valid material-name component via
378+
// materials.SanitizeMaterialName, falling back to "input" if nothing usable
379+
// remains.
377380
func sanitizeMaterialNamePart(s string) string {
378-
var b strings.Builder
379-
pendingHyphen := false
380-
for _, r := range strings.ToLower(s) {
381-
if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') {
382-
if pendingHyphen && b.Len() > 0 {
383-
b.WriteByte('-')
384-
}
385-
b.WriteRune(r)
386-
pendingHyphen = false
387-
} else {
388-
pendingHyphen = true
389-
}
390-
}
391-
392-
if b.Len() == 0 {
393-
return "input"
381+
if name := materials.SanitizeMaterialName(s); name != "" {
382+
return name
394383
}
395-
return b.String()
384+
return "input"
396385
}
397386

398387
// GetPolicyEvaluations is a Wrapper around the getPolicyEvaluations

‎app/cli/pkg/action/attestation_add_routing_test.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,19 @@ func TestShouldExplode(t *testing.T) {
5858
value string
5959
wantFormat materials.ArchiveFormat
6060
}{
61-
// A non-ArchiveNone format means the value will be exploded.
62-
{"kind + archive", "SBOM_CYCLONEDX_JSON", zipPath, materials.ArchiveZip},
63-
{"archive-native kind", "ZAP_DAST_ZIP", zipPath, materials.ArchiveNone},
61+
// A non-ArchiveNone format means the value will be exploded. Only
62+
// explodable kinds (SBOM, SARIF) explode; everything else is recorded
63+
// whole even when the value is an archive.
64+
{"explodable SBOM + archive", "SBOM_CYCLONEDX_JSON", zipPath, materials.ArchiveZip},
65+
{"explodable SARIF + archive", "SARIF", zipPath, materials.ArchiveZip},
66+
{"non-explodable ARTIFACT + archive", "ARTIFACT", zipPath, materials.ArchiveNone},
67+
{"non-explodable EVIDENCE + archive", "EVIDENCE", zipPath, materials.ArchiveNone},
68+
{"archive-native ZAP + archive", "ZAP_DAST_ZIP", zipPath, materials.ArchiveNone},
6469
{"no kind", "", zipPath, materials.ArchiveNone},
65-
{"kind + non-archive", "ARTIFACT", plainPath, materials.ArchiveNone},
66-
// Non-file values must never return an error — STRING and CONTAINER_IMAGE
67-
// carry values that are not file paths at all.
68-
{"kind STRING non-file value", "STRING", "hello world", materials.ArchiveNone},
69-
{"kind CONTAINER_IMAGE non-file value", "CONTAINER_IMAGE", "registry.example.com/app:v1", materials.ArchiveNone},
70+
{"explodable kind + non-archive", "SBOM_CYCLONEDX_JSON", plainPath, materials.ArchiveNone},
71+
// Non-file values must never return an error — even for an explodable kind
72+
// the value here is not a file path at all.
73+
{"explodable kind STRING-like non-file value", "SARIF", "hello world", materials.ArchiveNone},
7074
}
7175
for _, tc := range tests {
7276
t.Run(tc.name, func(t *testing.T) {

‎pkg/attestation/crafter/materials/archive.go‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -267,18 +267,21 @@ func walkTar(p string, gzipped bool, visit func(name string, r io.Reader) error)
267267
}
268268
}
269269

270-
// archiveNativeKinds lists material kinds whose value is the archive itself.
271-
// For these, --kind short-circuits the explode path and the archive is
272-
// recorded whole. Extend this set as new "the archive is the material" kinds
273-
// are added.
274-
var archiveNativeKinds = map[string]struct{}{
275-
schemaapi.CraftingSchema_Material_ZAP_DAST_ZIP.String(): {},
270+
// explodableKinds is the allowlist of material kinds whose archive value is
271+
// expanded into one material per entry. Every other kind (ARTIFACT, EVIDENCE,
272+
// ZAP_DAST_ZIP, …) records the archive whole, so a customer can still provide a
273+
// regular zip as a single material. Extend this set as more kinds gain a
274+
// meaningful "bundle of the same kind" archive form.
275+
var explodableKinds = map[string]struct{}{
276+
schemaapi.CraftingSchema_Material_SBOM_CYCLONEDX_JSON.String(): {},
277+
schemaapi.CraftingSchema_Material_SBOM_SPDX_JSON.String(): {},
278+
schemaapi.CraftingSchema_Material_SARIF.String(): {},
276279
}
277280

278-
// IsArchiveNativeKind reports whether kind treats the archive as a single
279-
// material (recorded whole) rather than something to explode.
280-
func IsArchiveNativeKind(kind string) bool {
281-
_, ok := archiveNativeKinds[kind]
281+
// IsExplodableKind reports whether an archive provided for kind should be
282+
// expanded into one material per entry (true) or recorded whole (false).
283+
func IsExplodableKind(kind string) bool {
284+
_, ok := explodableKinds[kind]
282285
return ok
283286
}
284287

@@ -295,9 +298,10 @@ func ArchiveEntryBaseName(name string) string {
295298
// (empty/symbol-only input or prefix).
296299
const defaultMaterialName = "material"
297300

298-
// SanitizeMaterialName converts s into a valid DNS-1123 material name:
299-
// lowercase, with every run of characters outside [a-z0-9] collapsed to a
300-
// single "-" and leading/trailing "-" trimmed. Falls back to "material".
301+
// SanitizeMaterialName converts s into a valid DNS-1123 material-name component:
302+
// lowercase, with every run of characters outside [a-z0-9] collapsed to a single
303+
// "-" and leading/trailing "-" trimmed. It returns "" when nothing usable
304+
// remains; callers supply their own fallback.
301305
func SanitizeMaterialName(s string) string {
302306
var b strings.Builder
303307
pendingHyphen := false
@@ -312,9 +316,6 @@ func SanitizeMaterialName(s string) string {
312316
pendingHyphen = true
313317
}
314318
}
315-
if b.Len() == 0 {
316-
return defaultMaterialName
317-
}
318319
return b.String()
319320
}
320321

@@ -341,8 +342,8 @@ func NewNameAllocator(existing []string) *NameAllocator {
341342
// the base "material" (so entries are named material-0, material-1, …).
342343
func (a *NameAllocator) AllocateSequential(prefix string) string {
343344
base := defaultMaterialName
344-
if prefix != "" {
345-
base = SanitizeMaterialName(prefix)
345+
if s := SanitizeMaterialName(prefix); s != "" {
346+
base = s
346347
}
347348

348349
for {

‎pkg/attestation/crafter/materials/archive_test.go‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,8 @@ func TestSanitizeMaterialName(t *testing.T) {
239239
{"scan.json", "scan-json"},
240240
{"results.XML", "results-xml"},
241241
{"weird__name!!", "weird-name"},
242-
{"___", "material"},
242+
{"___", ""}, // nothing usable -> empty; callers supply their own fallback
243+
{"", ""},
243244
}
244245
for _, tc := range tests {
245246
assert.Equal(t, tc.want, SanitizeMaterialName(tc.in))
@@ -272,8 +273,14 @@ func TestNameAllocatorSequential(t *testing.T) {
272273
})
273274
}
274275

275-
func TestIsArchiveNativeKind(t *testing.T) {
276-
assert.True(t, IsArchiveNativeKind("ZAP_DAST_ZIP"))
277-
assert.False(t, IsArchiveNativeKind("SBOM_CYCLONEDX_JSON"))
278-
assert.False(t, IsArchiveNativeKind("ARTIFACT"))
276+
func TestIsExplodableKind(t *testing.T) {
277+
// Explodable: SBOM and SARIF bundles.
278+
assert.True(t, IsExplodableKind("SBOM_CYCLONEDX_JSON"))
279+
assert.True(t, IsExplodableKind("SBOM_SPDX_JSON"))
280+
assert.True(t, IsExplodableKind("SARIF"))
281+
// Not explodable: recorded whole even when a zip/tar is provided.
282+
assert.False(t, IsExplodableKind("ARTIFACT"))
283+
assert.False(t, IsExplodableKind("EVIDENCE"))
284+
assert.False(t, IsExplodableKind("ZAP_DAST_ZIP"))
285+
assert.False(t, IsExplodableKind(""))
279286
}

0 commit comments

Comments
 (0)