feat(httpcapture): rename SandboxConfigResponse to CaptureConfigResponse, add dynamic capture toggles

[matthyx]

Summary

Implements the armoapi-go portion of ADR 0009 (§3, §4, §5, §6), plus follow-up changes from review (jnathangreeg, two rounds):

• Rename SandboxConfigResponse → CaptureConfigResponse, files renamed to match (sandboxconfig.go → captureconfig.go). Wire-neutral: CaptureConfig is inlined via anonymous-field promotion, so the JSON shape on the wire is unchanged; a stale importer gets a compile error, not a silent break.
• Add OTelEventsEnabled *bool to CaptureConfig — nil ⇒ allow (fail-safe), gates the new dynamic per-event OTel raw-event export filter.
• Add HTTPCaptureTapEnabled *bool to CaptureConfig — nil ⇒ deny (fail-closed), the dedicated backend-driven switch for whether the HTTP-capture tap is constructed at all, distinct from the existing Enabled field (a lossless per-transaction policy decision on an already-existing tap). Named with the package's own initialism convention (CaptureProtocolHTTP, CaptureProtocolHTTPS) — round 1 review caught the original Http... casing.
• Add WorkloadScanEnabled *bool to CaptureConfig — nil ⇒ deny (fail-closed), gates the NA-3 workload scanner.
• Add MergeWithGlobal *bool to CaptureConfig — nil ⇒ true. Fully wired up as of round 2: cadashboardbe's resolveHTTPCaptureConfig branches on it (wantsMerge) — explicit false on a customer document opts that customer out of merging entirely, returning it exactly as stored. Only meaningful on a customer document; setting it on the global document has no effect (documented in the contract).
• dynamictoggles_test.go: pins all four toggles are absent-when-nil, that an explicit false still serializes (omitempty elides only nil), and that nil-vs-false survives an actual JSON round trip.
• Merge granularity (which fields fall back per-field to global vs. come from the customer document as a whole, and why) is now documented in this repo's own contract doc, not only on the cadashboardbe side.

Placement note: all four new fields live on CaptureConfig, not on the response envelope, even though the ADR's prose describes the first three as living "on the envelope." CaptureConfig is what config-service persists and what cadashboardbe resolves per-customer-over-global; placing the fields on the envelope instead would leave them with no per-tenant storage path. This matches the existing placement of MaskKnownCredentialHeaders.

Rollout order (this matters — read before merging)

HTTPCaptureTapEnabled and WorkloadScanEnabled are nil ⇒ deny, and no stored document can carry a field that didn't exist when it was written. So:

1. This PR merges and is tagged.
2. config-service#449 bumps its armoapi-go dependency (both go.mod and the separately-versioned sdk/go.mod, which was 9 releases further behind) — without this, an admin write would silently drop the new fields on decode, not just fail to read them. The global default document must then be explicitly patched with the fields it needs (e.g. httpCaptureTapEnabled: true) — an absent field is indistinguishable from "not shipped yet" and resolves to deny.
3. cadashboardbe#3063 bumps to the tagged release (currently on this PR's pseudo-version) and carries the merge logic.
4. private-node-agent (companion PR) ships last — only once the backend can actually serve true for whichever tenants need the tap/scanner on, since the moment the agent build that reads these fields deploys, httpCaptureTapEnabled/workloadScanEnabled become the only way to enable the HTTP-capture tap / NA-3 scanner (no local override survives ADR 0009 §5/§6 — see that PR).

All four repos (this one, config-service#449, cadashboardbe#3063, private-node-agent) are currently pinned to the same armoapi-go pseudo-version (updated after each round of fixes here) via go.mod — they'll need to move to a real tagged version once this merges.

NA-3's gate is singular, not competing: private-node-agent's companion PR removes the local NA3_SCAN_ENABLED env var outright (not merely superseded) — WorkloadScanEnabled here is the only gate once that PR ships. No redeploy can silently override it.

armosec/backend-services: confirmed this is a separate, non-live-synced monorepo — its own README states each service is imported "at the commit of its latest release," with no continuous history, so it's a periodic snapshot tied to tagged releases, not something CI builds continuously against main. It will pick up this rollout automatically whenever cadashboardbe's next release gets re-imported there — no separate patch needed in this PR, but worth someone confirming that re-import happens after cadashboardbe#3063's fix lands, not before.

Downstream / companion PRs

• config-service#449: dependency bump only (both modules), so the fields are storable at all
• cadashboardbe#3063: CaptureConfigResponse rename + the customer/global merge logic (resolveHTTPCaptureConfig → mergeCaptureConfig, now branching on MergeWithGlobal)
• careportsreceiver#148 (merged): new generic route alongside the existing /cloud/v1/aiSandbox/config — independent, no type dependency (its handler is a byte passthrough)
• private-node-agent: full §1/§3/§4/§5/§6 implementation

Test plan

• go build ./...
• go vet ./...
• go test ./armotypes/httpcapture/...
• Downstream repos build against this branch via a go.mod pseudo-version pin until merged

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: c2b73200-4e11-41b1-970d-e368c0f11745

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Updates the HTTP-capture config contract types in armotypes/httpcapture to align with ADR 0009 by renaming the served envelope type and extending the persisted CaptureConfig schema with new backend-driven feature toggles, while keeping the served JSON shape for the envelope wire-neutral via embedding.

Changes:

• Renamed SandboxConfigResponse to CaptureConfigResponse (including method receiver and tests).
• Added three new dynamic toggle fields to CaptureConfig: OTelEventsEnabled, HttpCaptureTapEnabled, WorkloadScanEnabled (all *bool with documented nil semantics).
• Updated feature docs to reflect the new response type name (but requires follow-up fixes for link anchors and documenting the new fields).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
docs/features/tlsoffsets-record-contract.md	Updates served envelope type name reference (needs link-anchor update).
docs/features/httpcapture-config-contract.md	Renames the served envelope section/type name (needs documentation of newly added toggle fields).
armotypes/httpcapture/sandboxconfig.go	Renames the served envelope Go type and updates method receiver accordingly.
armotypes/httpcapture/sandboxconfig_test.go	Updates unit tests to use the new envelope type name.
armotypes/httpcapture/httpcapture.go	Extends CaptureConfig with new pointer-based dynamic toggles and their contract semantics.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Coverage Report for CI Build 33489183546

Warning

No base build found for commit 81cae0e on main.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 48.443%

Details

• Patch coverage: 1 of 1 lines across 1 file are fully covered (100%).

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →

---

Coverage Stats

Relevant Lines:	5171
Covered Lines:	2505
Line Coverage:	48.44%
Coverage Strength:	6.03 hits per line

---

💛 - Coveralls

[jnathangreeg]

Review — contract change looks right, rollout chain is the risk

Reviewed against the branch (go build ./..., go vet ./armotypes/httpcapture/..., go test ./armotypes/httpcapture/... — all clean locally) and traced every consumer of the renamed type across the org. The rename is genuinely wire-neutral (anonymous embedded field; the ,inline tag has an empty name so encoding/json promotes regardless of the type name — the ADR is right that the tag isn't what causes it), and the new bson tags will store flat because config-service embeds CaptureConfig with bson:",inline". Good.

Three things I'd want resolved before this is safe to build on, then some smaller ones.

1. config-service is missing from the companion list — and without it the two fail-closed gates can never be turned on

config-service/types/types.go:920 embeds httpcapture.CaptureConfig (json:",inline" bson:",inline") inside HTTPCaptureConfig. That's the write surface for v1_http_capture_config. So httpCaptureTapEnabled and workloadScanEnabled are unsettable until config-service bumps armoapi-go — and config-service isn't in the "Downstream / companion PRs" list.

Combine that with nil ⇒ deny and the ordering matters a lot: no stored document can carry a field that didn't exist when it was written, so the first private-node-agent build that reads these two fields disables the HTTP-capture tap and the NA-3 scanner for every tenant, and there is no way to switch them back on until config-service ships. That's the same class of blackout the careportsreceiver PR (#148) is deliberately avoiding on the route side.

Please add config-service to the companion list and put the rollout order in the PR body, roughly:

1. this PR merges
2. config-service bumps armoapi-go and the global doc is patched with explicit true
3. cadashboardbe bumps
4. only then ship the agent build that reads them

Worth stating explicitly too: NA-3 already has an independent env gate (NA3_SCAN_ENABLED, set in the daemonset env, not config). After this PR there are two gates that both have to be on — say which one is authoritative, or the next redeploy silently wins the argument.

2. MergeWithGlobal is a settable, persisted field that does nothing

It carries both json and bson tags, so an operator can POST mergeWithGlobal: false through config-service, read it back unchanged in a GET, and still get merged config — no error, no warning. A flag that round-trips but is inert is worse than a missing flag, because the round-trip is the confirmation the operator will trust.

The merge logic is already in the companion cadashboardbe PR, so honoring it there is a ~3-line if. Either do that, or drop the field from this PR and land it with the branch that reads it. I'd rather not have "NOT YET BRANCHED ON" in a shared wire contract.

3. Per-field vs per-document merge is undefined — and it matters because the polarities differ

If cadashboardbe's merge is per-field-when-non-nil, a customer doc that omits otelEventsEnabled inherits global: correct. If it resolves the customer document wholesale, that same omission lands on nil ⇒ allow and silently overrides a global false — re-enabling raw eBPF export for a tenant that opted out. httpCaptureTapEnabled springs the same trap in the other direction (silently denying a tenant whose global says yes).

Mixed polarity in one struct is defensible, but only if the merge granularity is part of the contract. This PR is the contract — please state it in docs/features/httpcapture-config-contract.md and have the cadashboardbe PR pin it with a test (customer doc omitting each field, global setting each field, both polarities).

4. Rename with no transitional alias — one consumer isn't in the companion list

Consumers I found:

• cadashboardbe/httphandlerv2/runtime/httpcaptureconfig.go:77 + httpcaptureconfig_merge_test.go ✅ listed
• private-node-agent/pkg/httpcaptureconfig/config.go:40 ✅ listed — note it's embedded, so this rename also changes the promoted field name in the agent's own type, not just an import
• armosec/backend-services → cadashboardbe/httphandlerv2/runtime/httpcaptureconfig.go ❌ not listed. Same module github.com/armosec/cadashboardbe, non-archived, pushed the same day as the standalone repo. Whichever of the two is canonical, both need the rename — worth confirming which one CI actually builds.

A one-liner would decouple the merge order entirely:

// Deprecated: use CaptureConfigResponse.
type SandboxConfigResponse = CaptureConfigResponse

The PR's argument ("a stale importer gets a compile error, not a silent break") holds either way — an alias still can't break silently, and staleness surfaces as a deprecation warning instead of a hard lockstep. Keeping it strict is a legitimate choice, but then the merge order for four repos belongs in the PR body.

5. HttpCaptureTapEnabled → HTTPCaptureTapEnabled

The package's own convention is the uppercased initialism: CaptureProtocolHTTP, CaptureProtocolHTTPS, TLSOffsets, and OTelEventsEnabled right above it. This is exported API in a shared module, so fixing it later is another breaking rename — do it now, while the same PR is already breaking a type name in this package. The wire keys (httpCaptureTapEnabled) stay exactly as they are.

6. No test covers any of the four new fields

The entire safety argument in this PR is polarity — nil vs explicit false vs true — and the test diff is purely the mechanical rename, which only re-proves what the compiler already enforces. One table test would protect the actual contract:

• all four keys absent from the wire when nil
• a pointer-to-false does serialize (omitempty elides only the nil pointer) — this is the assertion that stops a future "simplify *bool → bool" from silently erasing every explicit false in the fleet
• round-trip preserves nil vs false distinctly

Nit

sandboxconfig.go / sandboxconfig_test.go now define CaptureConfigResponse. If the files are getting renamed to match, this is the cheap window to do it.

[jnathangreeg]

Re-review — code side is resolved; the rollout runbook is the one thing still missing

Re-checked on 769dc8bd: go build ./..., go vet ./armotypes/httpcapture/..., go test ./armotypes/httpcapture/... all green. The old HttpCaptureTapEnabled spelling is gone everywhere in the module.

Resolved

Naming (my #5) — HTTPCaptureTapEnabled, wire key untouched. Right call to do it in the same breaking window.

Polarity tests (my #6) — dynamictoggles_test.go covers exactly the three states that matter, and the middle one is the important one: TestDynamicToggles_ExplicitFalseSerializes is what will fail loudly the day someone "simplifies" *bool → bool and silently erases every explicit false in the fleet. Round-trip through real marshal/unmarshal rather than struct assertions is the right level.

Merge granularity (my #3) — documented in the contract, and I checked it against the implementation rather than taking the doc's word: mergeCaptureConfig in cadashboardbe#3063 matches the documented split field-for-field (the four pointers via firstNonNilBool, everything else from merged := customer). The reasoning for the split — pointers have an unambiguous "not set", scalars can't distinguish a deliberate enabled: false from an omission — is the correct justification and worth having in the shared contract rather than only on the cadashboardbe side. The three new resolver tests cover pointer-fallback, customer-wins, and single-doc-no-merge.

MergeWithGlobal inertness (my #2) — genuinely wired now, not just re-worded: wantsMerge short-circuits resolveHTTPCaptureConfig, pinned by TestResolveHTTPCaptureConfig_MergeWithGlobalFalseOptsOut. The doc comment is now true, which is what I was after.

config-service (half of my #1) — config-service#449 exists and bumps both go.mod and sdk/go.mod (the latter was still on v0.0.750, nine releases behind), so the fields are writable and storable. That was the part I couldn't see when I first reviewed.

Still open

1. The PR body still lists three companions and no rollout order. config-service#449 exists but isn't referenced from here, and the ordering constraint isn't written down anywhere: httpCaptureTapEnabled and workloadScanEnabled are nil ⇒ deny, and no stored document can carry a field that didn't exist when it was written — so the global document has to be patched with explicit true before the private-node-agent build that reads them ships, or capture and NA-3 go dark fleet-wide.

Everything technical is now correct; what's missing is the runbook, and no test can enforce it. Please add config-service#449 to the companion list and spell out the order (armoapi-go tag → config-service bump + patch the global doc → cadashboardbe → agent). Also still worth one line on NA-3's two independent gates (NA3_SCAN_ENABLED in the daemonset env plus this field) and which is authoritative — a redeploy currently decides that argument silently.

2. Merge order is now concrete, so it belongs in the body too. Four repos pin v0.0.760-0.20260901125601-769dc8bda13b (cadashboardbe#3063, config-service#449, private-node-agent, and this). Without a transitional alias — your call, and I'm not relitigating it — this PR has to merge and tag before any of them can come off pseudo-versions. That's fine, it just needs saying out loud.

3. armosec/backend-services is still unaccounted for. #3063 patched armosec/cadashboardbe; backend-services carries its own cadashboardbe/httphandlerv2/runtime/httpcaptureconfig.go with the same module path, non-archived, pushed the same day. If CI builds that copy too, it breaks on the bump. Just needs someone to confirm which one is canonical.

Two small new ones, from reading the wired-up merge

4. wantsMerge only ever reads the customer document's MergeWithGlobal. A global document with mergeWithGlobal: false is silently ignored — which is a reasonable design (reading it from global would be circular), but it's also exactly what someone will try first when they want to disable merging fleet-wide. One sentence in the contract doc saying the field is only meaningful on a customer document would save that trip.

5. Stale line in cadashboardbe#3063's doc (flagging here since it's this PR's rename that stales it): it says CaptureConfig "gained three new pointer-bool fields (OTelEventsEnabled, HttpCaptureTapEnabled, WorkloadScanEnabled)" — four fields now, and the Go name is HTTPCaptureTapEnabled after this commit.

Nit from last time, still true and still cheap: sandboxconfig.go / _test.go now define CaptureConfigResponse. Same breaking window, so it's now or it stays mismatched for a long time.

[matthyx]

Addressed all 5 open items from the re-review:

1 & 2 (companion list + rollout order): PR body now lists config-service#449 explicitly and spells out the merge order (armoapi-go → config-service#449 + patch the global doc → cadashboardbe#3063 → private-node-agent). Also clarified NA-3's gate is singular after the private-node-agent PR ships — NA3_SCAN_ENABLED is removed outright there, not left as a second competing gate.

3 (armosec/backend-services): Checked its README directly — it imports each service "at the commit of its latest release," with no continuous history (one commit per service, re-imported only on a new release tag). It's not something CI builds continuously against main, so it won't break on this rollout; it'll just need re-importing after cadashboardbe's next release once #3063 lands. Noted in the PR body so it's not a silent gap.

4 (MergeWithGlobal only meaningful on the customer doc): Added the one sentence to the contract doc — resolveHTTPCaptureConfig never reads it off the global document (would be circular).

5 (stale doc line in cadashboardbe#3063 + the file rename nit): Fixed both — cadashboardbe's doc now says four fields with the corrected HTTPCaptureTapEnabled casing, and sandboxconfig.go/_test.go are renamed to captureconfig.go/_test.go in this same breaking window, pushed as b3e0e60.

All four repos (this one, config-service#449, cadashboardbe#3063, private-node-agent) are now re-pinned to b3e0e60 so the hashes you're tracking stay in sync.

[jnathangreeg]

Round 3 — all my review items are closed

Verified on b3e0e60: files renamed to captureconfig.go/_test.go, and the contract now states that mergeWithGlobal is only meaningful on a customer document (with the reason — reading it off global would be circular). go build/vet/test green.

The PR body now carries the rollout order, config-service#449 in the companion list, and answers both open questions from my last pass:

• NA-3's two gates — resolved by removing NA3_SCAN_ENABLED outright rather than layering. I confirmed that in private-node-agent#562: the env var is gone from the loader, and pkg/workloadscanner/scanner.go's remaining check is a defensive backstop, not a gate. Singular gate, as claimed.
• armosec/backend-services — accepting the snapshot-at-release finding; that matches what I could see of the repo. The one thing that still needs a human: making sure the re-import happens after cadashboardbe#3063's merge-fallback fix lands, not before.

I also confirmed config-service#449's pin (v0.0.760-0.20260901143018-b3e0e60ab086) matches this branch's head exactly, so the four repos are genuinely on the same commit rather than drifting pseudo-versions.

Nothing outstanding from me on this PR. Merge order is the only remaining constraint: this tags first, then #449 repins, then #3063.