🎯 Code Coverage (details)
• Patch Coverage: 44.44%
• Overall Coverage: 50.29% (-0.02%)

I think this cache is a good local improvement, but it looks like we are still parsing Terraform twice in the full scan pipeline.

The first parse happens during ingestion in pkg/parser/terraform/terraform.go:227 (Parser.Parse). The actual HCL parse is at pkg/parser/terraform/terraform.go:240:

file, diagnostics := hclsyntax.ParseConfig(resolved, filepath.Base(path), hcl.Pos{Byte: 0, Line: 1, Column: 1})

That parsed *hcl.File / *hclsyntax.Body is then converted into a generic model.Document at pkg/parser/terraform/terraform.go:259-260:

fc, parseErr := p.convertFunc(ctx, file, inputVariables)
json, err := addExtraInfo(ctx, []model.Document{fc}, path)

After Parser.Parse returns, the HCL syntax tree is not retained. runner.sink starts at pkg/runner/sink.go:32, calls the parser at pkg/runner/sink.go:50, and then stores only the JSON-like document and source text in model.FileMetadata at pkg/runner/sink.go:89-100:

Document:          preparedDocument,
LineInfoDocument:  document,
OriginalData:      documents.Content,
LinesOriginalData: utils.SplitLines(documents.Content),

Later, when building vulnerability line/block locations, pkg/detector/terraform/terraform_detect.go:59 (DetectLine) needs HCL range information. The new cache/reparse call is at pkg/detector/terraform/terraform_detect.go:109, and block lookup happens in locateTerraformBlock at pkg/detector/terraform/terraform_detect.go:148. That code uses range information at pkg/detector/terraform/terraform_detect.go:162-168, such as:

block.TypeRange.Start
block.Body.SrcRange.End
attr.SrcRange.Start
attr.SrcRange.End

Attribute ranges are also used when finding nested structures at pkg/detector/terraform/terraform_detect.go:258-270.

Those ranges are not available from model.Document, _dd_lines, OriginalData, or ResolvedFiles. FileMetadata is defined at pkg/model/model.go:154-168, and ResolvedFile only stores path/line content at pkg/model/model.go:360-363; it does not carry the HCL AST. Because of that, the detector reparses the file with hclsyntax.ParseConfig. This PR improves that by caching the second parse per file, but it does not eliminate the duplicate parse between ingestion and line detection.

Could we investigate a one-time parse design? For example, the Terraform parser could retain/share the parsed *hclsyntax.Body in a per-scan sidecar cache keyed by file ID/content/path, and the Terraform detector could read from that cache instead of reparsing OriginalData.

A few things to be careful about:

• The parser interface is shared by all languages (pkg/parser/parser.go:26-34), so Terraform-specific syntax data should probably be optional/internal rather than part of the OPA Document.
• The HCL AST should not be included in Document, reports, or OPA input.
• The detector needs ranges matching the user-visible source text. Today Terraform Resolve returns the original content at pkg/parser/terraform/terraform.go:73-85, but if that ever changes, we need to ensure the cached AST and OriginalData/LinesOriginalData refer to the same bytes.
• The cache key should avoid stale parses if an Inspector or detector is reused across scans. A content hash or file ID + content hash may be safer than path alone.

So the current PR is useful, but it may be worth exploring whether the first Terraform parse can be preserved and reused for block/remediation location detection.

I checked the server mode path for the Terraform HCL cache lifetime.

Current behavior looks safe. Even though serve is a long-lived process, it does not keep a long-lived Inspector between /ide/v1/iac/analyze calls. Each analyze request builds a new request-local scan object graph:

• Server.analyze starts the request scan path at pkg/server/analyze.go:162.
• Server.analyze builds a new in-memory filesystem from the pushed request files at pkg/server/analyze.go:167.
• Server.analyze creates a new scan.Client at pkg/server/analyze.go:209.
• Client.Scan calls executeScan at pkg/scan/client.go:189.
• executeScan calls initScan at pkg/scan/scan.go:160.
• initScan creates a new engine.NewInspector at pkg/scan/scan.go:88.
• NewInspector creates a new hclcache.Cache at pkg/engine/inspector.go:195.
• NewInspector stores that cache on the inspector at pkg/engine/inspector.go:208.
• createService wires the same inspector cache into the Terraform parser at pkg/scan/scan.go:278 and pkg/scan/scan.go:280.

So with the current architecture, the Terraform HCL cache is effectively scoped to one analyze request / one scan. A second server analyze request with the same file path but changed content should not see a stale *hclsyntax.Body from the previous request.

That said, I think we should add a regression test in pkg/server/analyze_test.go to lock this behavior down. The important invariant is: the HCL cache must not persist between two server analyze requests.

A good test would:

1. Send an analyze request with infra/main.tf containing a Terraform resource.
2. Send a second analyze request to the same server with the same infra/main.tf path but different content, ideally moving/changing the resource block enough that stale HCL ranges would be visible.
3. Assert that the second finding's line, block location, or resource source comes from the second request's content, not from the first request's cached parse.

This matters for future server optimizations. Keeping the inspector or rules warm between requests would be attractive for latency, but with the current path-only cache key it could make stale parses possible unless the cache is also reset or keyed by content.

Side note, not mandatory for this PR: I am also a little worried about the memory footprint. The cache stores one parsed HCL body per Terraform file for the duration of the scan/request. For large repos this may be fine, but a lightweight metric or debug log could be useful later, for example number of cached HCL bodies, cache hit/miss count, and maybe total source bytes represented. That would help us validate the CPU win against the memory cost.

I checked the server mode path for the Terraform HCL cache lifetime.

Current behavior looks safe. Even though serve is a long-lived process, it does not keep a long-lived Inspector between /ide/v1/iac/analyze calls. Each analyze request builds a new request-local scan object graph:

• Server.analyze starts the request scan path at pkg/server/analyze.go:162.
• Server.analyze builds a new in-memory filesystem from the pushed request files at pkg/server/analyze.go:167.
• Server.analyze creates a new scan.Client at pkg/server/analyze.go:209.
• Client.Scan calls executeScan at pkg/scan/client.go:189.
• executeScan calls initScan at pkg/scan/scan.go:160.
• initScan creates a new engine.NewInspector at pkg/scan/scan.go:88.
• NewInspector creates a new hclcache.Cache at pkg/engine/inspector.go:195.
• NewInspector stores that cache on the inspector at pkg/engine/inspector.go:208.
• createService wires the same inspector cache into the Terraform parser at pkg/scan/scan.go:278 and pkg/scan/scan.go:280.

So with the current architecture, the Terraform HCL cache is effectively scoped to one analyze request / one scan. A second server analyze request with the same file path but changed content should not see a stale *hclsyntax.Body from the previous request.

That said, I think we should add a regression test in pkg/server/analyze_test.go to lock this behavior down. The important invariant is: the HCL cache must not persist between two server analyze requests.

A good test would:

1. Send an analyze request with infra/main.tf containing a Terraform resource.
2. Send a second analyze request to the same server with the same infra/main.tf path but different content, ideally moving/changing the resource block enough that stale HCL ranges would be visible.
3. Assert that the second finding's line, block location, or resource source comes from the second request's content, not from the first request's cached parse.

This matters for future server optimizations. Keeping the inspector or rules warm between requests would be attractive for latency, but with the current path-only cache key it could make stale parses possible unless the cache is also reset or keyed by content.

Side note, not mandatory for this PR: I am also a little worried about the memory footprint. The cache stores one parsed HCL body per Terraform file for the duration of the scan/request. For large repos this may be fine, but a lightweight metric or debug log could be useful later, for example number of cached HCL bodies, cache hit/miss count, and maybe total source bytes represented. That would help us validate the CPU win against the memory cost.

@NathanGallet-dd

Thanks for the review.

On the ingestion-sharing idea (reusing the parser's HCL AST instead of reparsing in the detector): I prototyped it and benchmarked all three variants on cloud-inventory with a full-platform scan:

The lazy detector-side cache (DetectKindLine with a per-scan sync.Map, populated on first finding per file) captures essentially all of the CPU win. Ingestion sharing only eliminates the single remaining parse for files that already have findings, but it retains parsed bodies for every ingested .tf file (including ~20k+ with zero findings). That added ~540 MB peak RSS with no measurable further speedup (actually slightly slower in our runs).

So I dropped the ingestion-sharing commit and kept the original logic.

The ingestion sharing is logical if the bottleneck is “we parse every file twice.” After the lazy cache, the bottleneck is gone; what’s left is “we parse files-with-findings one extra time on the first finding,” which is cheap. Paying to retain every file’s AST up front buys almost no CPU and costs a lot of RAM.

On the optional hit/miss/byte telemetry, agree it would be useful for operational visibility; happy to follow up separately if we want it.