diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state.go index 1a8412883..fa1c5365d 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state.go @@ -239,7 +239,7 @@ func (m *Attestation_Material) ingestMaterialToJSON(rawMaterial []byte, value st // AccessChk emits plain text; project it to JSON so the policy engine, // which only consumes JSON, can evaluate it. The raw text is preserved // in the projection's "raw" field for string-matching fallbacks. - report, err := accesschk.Parse(rawMaterial) + report, err := accesschk.Parse(bytes.NewReader(rawMaterial)) if err != nil { return nil, fmt.Errorf("invalid accesschk material: %w", err) } diff --git a/pkg/attestation/crafter/materials/accesschk.go b/pkg/attestation/crafter/materials/accesschk.go index c9d97d249..e2b6890b3 100644 --- a/pkg/attestation/crafter/materials/accesschk.go +++ b/pkg/attestation/crafter/materials/accesschk.go @@ -44,16 +44,18 @@ func NewAccessChkCrafter(schema *schemaapi.CraftingSchema_Material, backend *cas } func (i *AccessChkCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { - data, err := os.ReadFile(filePath) + f, err := os.Open(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) } + defer func() { _ = f.Close() }() // Soft fingerprint: AccessChk emits free-form text, so we only require that // the input is valid text that resembles AccessChk output (a banner, at - // least one access entry, or an SDDL/descriptor marker). The raw text is - // stored unchanged; it is projected to JSON later for policy evaluation. - report, err := accesschk.Parse(data) + // least one access entry, or an SDDL/descriptor marker). The file is streamed + // rather than read whole into memory; it is stored unchanged by the uploader + // below and projected to JSON later for policy evaluation. + report, err := accesschk.Parse(f) if err != nil { return nil, fmt.Errorf("invalid accesschk output: %w", ErrInvalidMaterialType) } @@ -62,18 +64,6 @@ func (i *AccessChkCrafter) Craft(ctx context.Context, filePath string) (*api.Att return nil, fmt.Errorf("input does not look like accesschk output: %w", ErrInvalidMaterialType) } - // The material is attested by digest and stored as-is, but at policy - // evaluation time it is projected to JSON client-side on the runner. Above - // this size the parser omits the verbatim raw-text fallback to keep peak - // memory bounded; warn so an operator relying on string-matching policies is - // aware the fallback fields will not be present for this material. - if len(data) > accesschk.RawRetentionLimit { - i.logger.Warn(). - Int("size", len(data)). - Int("threshold", accesschk.RawRetentionLimit). - Msg("large AccessChk material: raw-text fallback fields are omitted from the policy input to limit memory use") - } - m, err := uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger) if err != nil { return nil, err diff --git a/pkg/attestation/crafter/materials/accesschk/accesschk.go b/pkg/attestation/crafter/materials/accesschk/accesschk.go index 344d3212f..1ac6e6a26 100644 --- a/pkg/attestation/crafter/materials/accesschk/accesschk.go +++ b/pkg/attestation/crafter/materials/accesschk/accesschk.go @@ -21,17 +21,16 @@ // text is retained in Raw so a policy can fall back to string matching // regardless of the output mode used. // -// The parser streams the input line by line rather than building a normalized -// full-text copy, and it size-gates the verbatim fallback fields (Raw and the -// per-object RawLines). Both measures keep peak memory bounded: a -// several-hundred-MB material would otherwise pin multiple copies of itself in -// memory and double the JSON document handed to the policy engine, which has -// OOM-killed CI runners during client-side policy evaluation. +// Parse reads from an io.Reader and streams the input line by line rather than +// buffering the whole material in memory or building a normalized full-text +// copy, and it size-gates the verbatim fallback fields (Raw and the per-object +// RawLines). Together these keep peak memory bounded: a large material would +// otherwise pin multiple copies of itself in memory and inflate the JSON +// document handed to the policy engine to multiples of the input size. package accesschk import ( "bufio" - "bytes" "fmt" "io" "regexp" @@ -108,35 +107,45 @@ type Object struct { // Raw holds the full original text for inputs below RawRetentionLimit and is // empty otherwise; descriptorMarker records whether an SDDL/descriptor marker // was seen during parsing so LooksLikeAccessChk stays reliable even when Raw is -// omitted for oversized inputs. +// omitted for oversized inputs. rawOmitted records whether the verbatim fallback +// fields were dropped because the input exceeded RawRetentionLimit, so callers +// can warn without knowing the input size upfront (see RawOmitted). type Report struct { Tool Tool `json:"tool"` Objects []Object `json:"objects"` Raw string `json:"raw"` descriptorMarker bool + rawOmitted bool } -// Parse converts AccessChk text output into a Report. It only returns an error -// when the input is not valid UTF-8 text; well-formed text always parses, with +// RawOmitted reports whether the verbatim fallback fields (Raw and the per-object +// RawLines) were dropped because the input exceeded RawRetentionLimit. It lets a +// streaming caller warn about the omission without measuring the input itself. +func (r *Report) RawOmitted() bool { + return r.rawOmitted +} + +// Parse converts AccessChk text output read from r into a Report. It streams r +// and never buffers the whole input, so it can parse a file handle directly +// without an intermediate full-file copy. It only returns an error when the +// input is not valid UTF-8 text or r fails; well-formed text always parses, with // any unrecognized content preserved in the per-object RawLines and the // top-level Raw field for inputs below RawRetentionLimit (see the package doc). -func Parse(data []byte) (*Report, error) { - if !utf8.Valid(data) { - return nil, fmt.Errorf("input is not valid UTF-8 text") - } - - // Retain the verbatim fallback fields only for inputs small enough that the - // resulting JSON projection stays bounded; see RawRetentionLimit. - retainRaw := len(data) <= RawRetentionLimit - +func Parse(r io.Reader) (*Report, error) { report := &Report{ Tool: Tool{Name: ToolName}, Objects: []Object{}, } - if retainRaw { - report.Raw = string(data) - } + + // Retain the verbatim fallback fields (Raw, RawLines) optimistically. The + // input size is unknown when streaming, so accumulate while the running byte + // count stays within RawRetentionLimit; the moment it crosses, drop + // everything retained so far and stop, keeping peak memory bounded for + // oversized materials without needing the size upfront. + retainRaw := true + var rawBuilder strings.Builder + var total int var current *Object var entryIndent int @@ -147,12 +156,37 @@ func Parse(data []byte) (*Report, error) { // at once. bufio.Reader.ReadString grows to fit arbitrarily long lines and // returns freshly allocated strings, so stored substrings do not pin the // entire input in memory. - reader := bufio.NewReader(bytes.NewReader(data)) + reader := bufio.NewReader(r) for { raw, readErr := reader.ReadString('\n') + total += len(raw) + + // Validate UTF-8 incrementally. A '\n' (0x0A) can never be part of a + // multi-byte UTF-8 sequence, so splitting on it never splits a rune and + // per-line validation is equivalent to validating the whole input. + if !utf8.ValidString(raw) { + return nil, fmt.Errorf("input is not valid UTF-8 text") + } + line := strings.TrimSuffix(raw, "\n") line = strings.TrimSuffix(line, "\r") + // Cross the retention limit: discard everything retained so far (Raw and + // every object's RawLines) and stop retaining. This is all-or-nothing so + // oversized reports never carry a partial verbatim copy. + if retainRaw && total > RawRetentionLimit { + retainRaw = false + report.rawOmitted = true + rawBuilder.Reset() + for i := range report.Objects { + report.Objects[i].RawLines = nil + } + } + // Concatenating every ReadString chunk reproduces the input verbatim. + if retainRaw { + rawBuilder.WriteString(raw) + } + if line != "" || readErr == nil { processLine(report, line, ¤t, &entryIndent, §ion, ¤tACE, retainRaw) } @@ -165,6 +199,10 @@ func Parse(data []byte) (*Report, error) { } } + if retainRaw { + report.Raw = rawBuilder.String() + } + return report, nil } diff --git a/pkg/attestation/crafter/materials/accesschk/accesschk_test.go b/pkg/attestation/crafter/materials/accesschk/accesschk_test.go index ca4526b75..32ade9fe0 100644 --- a/pkg/attestation/crafter/materials/accesschk/accesschk_test.go +++ b/pkg/attestation/crafter/materials/accesschk/accesschk_test.go @@ -16,6 +16,7 @@ package accesschk_test import ( + "bytes" "os" "testing" @@ -28,7 +29,7 @@ func TestParse_Default(t *testing.T) { data, err := os.ReadFile("./testdata/default.txt") require.NoError(t, err) - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) assert.Equal(t, "AccessChk", report.Tool.Name) @@ -60,7 +61,7 @@ func TestParse_Verbose(t *testing.T) { data, err := os.ReadFile("./testdata/verbose.txt") require.NoError(t, err) - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) require.Len(t, report.Objects, 1) @@ -80,7 +81,7 @@ func TestParse_Service(t *testing.T) { data, err := os.ReadFile("./testdata/service.txt") require.NoError(t, err) - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) require.Len(t, report.Objects, 1) @@ -94,7 +95,7 @@ func TestParse_SDDL(t *testing.T) { data, err := os.ReadFile("./testdata/sddl.txt") require.NoError(t, err) - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) require.Len(t, report.Objects, 1) @@ -112,7 +113,7 @@ func TestParse_NoBanner(t *testing.T) { data, err := os.ReadFile("./testdata/nobanner.txt") require.NoError(t, err) - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) assert.Equal(t, "AccessChk", report.Tool.Name) @@ -127,7 +128,7 @@ func TestParse_DescriptorFormat(t *testing.T) { data, err := os.ReadFile("./testdata/descriptor.txt") require.NoError(t, err) - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) assert.True(t, report.LooksLikeAccessChk()) @@ -168,19 +169,42 @@ func TestParse_Garbage(t *testing.T) { data, err := os.ReadFile("./testdata/garbage.txt") require.NoError(t, err) - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) assert.False(t, report.LooksLikeAccessChk()) } func TestParse_InvalidUTF8(t *testing.T) { - _, err := accesschk.Parse([]byte{0xff, 0xfe, 0x00, 0x01}) + _, err := accesschk.Parse(bytes.NewReader([]byte{0xff, 0xfe, 0x00, 0x01})) assert.Error(t, err) } func TestParse_Empty(t *testing.T) { - report, err := accesschk.Parse([]byte{}) + report, err := accesschk.Parse(bytes.NewReader([]byte{})) require.NoError(t, err) assert.False(t, report.LooksLikeAccessChk()) assert.Empty(t, report.Objects) } + +// TestParse_StreamsFromOpenFile verifies Parse consumes an *os.File directly, +// i.e. it can stream from disk without the caller reading the whole file into +// memory first. +func TestParse_StreamsFromOpenFile(t *testing.T) { + f, err := os.Open("./testdata/default.txt") + require.NoError(t, err) + defer f.Close() + + report, err := accesschk.Parse(f) + require.NoError(t, err) + + assert.Equal(t, "6.15", report.Tool.Version) + assert.False(t, report.RawOmitted()) + require.Len(t, report.Objects, 1) + assert.Equal(t, `c:\windows\system32\notepad.exe`, report.Objects[0].Name) + require.Len(t, report.Objects[0].AccessEntries, 3) + + // Streaming still reconstructs the verbatim Raw fallback for small inputs. + raw, err := os.ReadFile("./testdata/default.txt") + require.NoError(t, err) + assert.Equal(t, string(raw), report.Raw) +} diff --git a/pkg/attestation/crafter/materials/accesschk/gate_test.go b/pkg/attestation/crafter/materials/accesschk/gate_test.go index 34352efd5..0415a6861 100644 --- a/pkg/attestation/crafter/materials/accesschk/gate_test.go +++ b/pkg/attestation/crafter/materials/accesschk/gate_test.go @@ -16,6 +16,7 @@ package accesschk_test import ( + "bytes" "strings" "testing" @@ -53,9 +54,10 @@ func TestParse_LargeInputOmitsRawFields(t *testing.T) { data := buildLargeInput(accesschk.RawRetentionLimit + 1) require.Greater(t, len(data), accesschk.RawRetentionLimit) - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) + assert.True(t, report.RawOmitted(), "RawOmitted must report the drop above the retention limit") assert.Empty(t, report.Raw, "Raw must be omitted above the retention limit") require.NotEmpty(t, report.Objects) for _, obj := range report.Objects { @@ -74,9 +76,10 @@ func TestParse_LargeInputOmitsRawFields(t *testing.T) { func TestParse_SmallInputRetainsRawFields(t *testing.T) { data := []byte("c:\\file\n RW BUILTIN\\Administrators\n") - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) + assert.False(t, report.RawOmitted(), "RawOmitted must be false below the retention limit") assert.Equal(t, string(data), report.Raw) require.Len(t, report.Objects, 1) assert.Equal(t, []string{" RW BUILTIN\\Administrators"}, report.Objects[0].RawLines) @@ -97,7 +100,7 @@ func TestParse_LargeSDDLStillDetected(t *testing.T) { data := []byte(b.String()) require.Greater(t, len(data), accesschk.RawRetentionLimit) - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) assert.Empty(t, report.Raw) @@ -109,7 +112,7 @@ func TestParse_LargeSDDLStillDetected(t *testing.T) { func TestParse_CRLFLineEndings(t *testing.T) { data := []byte("c:\\file\r\n RW Everyone\r\n") - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) require.Len(t, report.Objects, 1) @@ -126,7 +129,7 @@ func TestParse_VeryLongLine(t *testing.T) { principal := strings.Repeat("A", 128*1024) data := []byte("c:\\file\n RW " + principal + "\n") - report, err := accesschk.Parse(data) + report, err := accesschk.Parse(bytes.NewReader(data)) require.NoError(t, err) require.Len(t, report.Objects, 1)