Skip to content

Commit cbfaf80

Browse files
myasnikovdaniilmyasnikovdaniil
committed
fix(ci): refuse a manifest past the API gate's size cap
A first-party CRD padded with comments past 8 MiB was skipped by the detector and read as "no API change" while Helm still shipped it. No YAML file in the tree comes near the cap, so nothing legitimate is refused. A candidate past the cap is now an error, like every other file the gate cannot inspect. Measuring a link by the file it leads to, as the symlink handling does, would otherwise have turned a padded manifest behind a link from detected into unseen. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent f008468 commit cbfaf80

3 files changed

Lines changed: 21 additions & 19 deletions

File tree

‎internal/apigate/parse_test.go‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -402,12 +402,14 @@ spec: {group: metrics.k8s.io, version: v1beta1}
402402
}
403403
}
404404

405-
// TestLoadSnapshotSkipsOversizedFiles guards the size cap added alongside
405+
// TestLoadSnapshotRejectsOversizedFiles guards the size cap added alongside
406406
// content-based discovery: since every .yaml/.yml file in the tree is now a
407407
// candidate (no directory allowlist), an untrusted PR head checkout could
408408
// otherwise force an unbounded read/parse of a pathologically large file. A
409-
// first-party CRD past maxYAMLFileSize must be excluded from discovery.
410-
func TestLoadSnapshotSkipsOversizedFiles(t *testing.T) {
409+
// file past maxYAMLFileSize is an error, not a skipped file: a first-party CRD
410+
// padded with comments past the cap would otherwise read as "no API change"
411+
// while Helm still ships it.
412+
func TestLoadSnapshotRejectsOversizedFiles(t *testing.T) {
411413
root := t.TempDir()
412414
write := func(rel, content string) {
413415
p := filepath.Join(root, filepath.FromSlash(rel))
@@ -420,16 +422,16 @@ func TestLoadSnapshotSkipsOversizedFiles(t *testing.T) {
420422
}
421423
padding := strings.Repeat("# padding\n", (maxYAMLFileSize/len("# padding\n"))+1)
422424
write("packages/system/oversized/crds/widgets.yaml", padding+sampleCRD)
423-
// A small, legitimate resource so the empty-snapshot guard doesn't fire
424-
// for an unrelated reason.
425+
// A small, legitimate resource so the empty-snapshot guard cannot be the
426+
// reason for the error.
425427
write("packages/system/postgres-rd/cozyrds/postgres.yaml", sampleCozyRD)
426428

427429
snap, err := LoadSnapshot(root)
428-
if err != nil {
429-
t.Fatal(err)
430+
if err == nil {
431+
t.Fatalf("a file past maxYAMLFileSize was skipped: %v", snap)
430432
}
431-
if _, ok := snap[resourceKey{Group: "example.cozystack.io", Plural: "widgets"}]; ok {
432-
t.Fatalf("CRD in a file past maxYAMLFileSize must be excluded: %v", snap)
433+
if !strings.Contains(err.Error(), "past the") {
434+
t.Fatalf("unexpected error: %v", err)
433435
}
434436
}
435437

‎internal/apigate/snapshot.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,13 @@ func LoadSnapshot(dir string) (Snapshot, error) {
226226
// untrusted PR head checkouts with no other size guard; real chart
227227
// CRD/APIService manifests in this repo are well under a megabyte, so this
228228
// is generous headroom, not a tight fit, and only bounds a pathological or
229-
// adversarial file.
229+
// adversarial file. A file past it fails the run rather than being skipped:
230+
// Helm ships a manifest of any size, so one padded with comments past the cap
231+
// would otherwise read as "no API change".
230232
const maxYAMLFileSize = 8 << 20 // 8 MiB
231233

232234
// discoverYAMLFiles walks the crdRoots under the checkout and returns the path
233-
// of every .yaml/.yml file found, up to maxYAMLFileSize. Filtering to the CRD /
235+
// of every .yaml/.yml file found, none past maxYAMLFileSize. Filtering to the CRD /
234236
// APIService documents we actually care about happens at parse time in
235237
// LoadSnapshot.
236238
//
@@ -284,7 +286,7 @@ func (c checkout) discoverYAMLFiles() ([]string, error) {
284286
return nil
285287
}
286288
if size > maxYAMLFileSize {
287-
return nil
289+
return fmt.Errorf("%s is %d bytes, past the %d the gate reads; a manifest it skipped would read as no API change", rel(c.dir, path), size, maxYAMLFileSize)
288290
}
289291
out = append(out, path)
290292
return nil

‎internal/apigate/snapshot_test.go‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -185,17 +185,15 @@ func TestLoadSnapshotThroughSymlinkedCheckoutRoot(t *testing.T) {
185185
}
186186
}
187187

188-
func TestLoadSnapshotSkipsOversizedFileBehindLink(t *testing.T) {
188+
func TestLoadSnapshotRejectsOversizedFileBehindLink(t *testing.T) {
189189
root := t.TempDir()
190190
writeTree(t, root, "packages/system/widget-rd/cozyrds/widget.yaml", sampleCozyRD)
191191
writeTree(t, root, "packages/chart/big.txt", sampleCRD+"# "+strings.Repeat("x", maxYAMLFileSize)+"\n")
192192
linkTree(t, root, "packages/chart/crds/resource.yaml", "../big.txt")
193193

194-
snap, err := LoadSnapshot(root)
195-
if err != nil {
196-
t.Fatal(err)
197-
}
198-
if _, ok := snap[widgetsKey]; ok {
199-
t.Fatal("the size cap measured the link, not the file behind it")
194+
// Measured on the file behind the link, and refused, not skipped: before
195+
// the cap was an error a link to a padded manifest read as "no API change".
196+
if _, err := LoadSnapshot(root); err == nil || !strings.Contains(err.Error(), "past the") {
197+
t.Fatalf("an oversized file behind a link was not refused: %v", err)
200198
}
201199
}

0 commit comments

Comments
 (0)