feat: wire live Configuration into analytics instrumentation - #7133
feat: wire live Configuration into analytics instrumentation#7133nick-y-snyk wants to merge 10 commits into
Conversation
sendInstrumentation now passes analytics.WithConfiguration(eng.GetConfiguration()) alongside the existing WithLogger option, so GetV2InstrumentationObject's new shape-based secret scrubbing (go-application-framework#704) runs on real CLI invocations, not just library callers who opt in manually. Blocked from building on this branch alone until go-application-framework#704 merges and is tagged, then go.mod is bumped for real — verified locally against a temporary `replace github.com/snyk/go-application-framework => ../../go-application-framework` pointing at that PR's branch, not included in this commit.
|
| Status | Scan Engine |  Critical |
 High |
 Medium |
 Low |
Total (0) |
|---|---|---|---|---|---|---|
| Open Source Security | 0 | 0 | 0 | 0 | See details | |
| Licenses | 0 | 0 | 0 | 0 | See details | |
| ✅ | Code Security | 0 | 0 | 0 | 0 | 0 issues |
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.
Extracts the debug-gated unknown-arg/env-value computation into populateRedactionTerms and calls it regardless of debugEnabled, setting configuration.REDACTION_TERMS so the analytics scrub chokepoint can see these terms even on non-debug runs. writeLogHeader/AddTermsToReplace keep their exact existing debugEnabled gate.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
GAF moved REDACTION_TERMS from pkg/configuration to pkg/logging (its only reader) per PR #704 review feedback. Follows that rename here.
…tion-configuration' into feat/agent-feedback-secret-redaction-configuration
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Pulls in the analytics extension chokepoint fix (#704): scrub secret-shaped extension values before marshaling, not the marshaled JSON bytes, with cycle-guard for self-referential extension maps.
This comment has been minimized.
This comment has been minimized.
go get left stale v0.14.3 hash entries behind; go mod tidy prunes them.
This comment has been minimized.
This comment has been minimized.
| sendInstrumentation(context.Background(), mockEngine, instrumentor, &logger) | ||
| } | ||
|
|
||
| func Test_addClientMachineId(t *testing.T) { |
There was a problem hiding this comment.
Could you extend this test to verify the machine ID is not redacted by the new logic? More precisely: this test does not use the new scrubbing logic. It may miss the machine ID being replaced
populateRedactionTerms sweeps os.Environ() for unrecognized values and writes them to logging.REDACTION_TERMS, which the analytics scrub chokepoint (GAF #704) then redacts wherever it finds them verbatim. studio::client_machine_id and persona.agent both echo a raw env var value (INTERNAL_SNYK_CLIENT_MACHINE_ID, AI_AGENT) straight into an extension, so without this exclusion the chokepoint strips its own legitimate data back out as "***". Addresses review comment on #7133.
This comment has been minimized.
This comment has been minimized.
|
/describe |
|
PR Description updated to latest commit (8c90385) |
what about this @nick-y-snyk |
no longer relevant, pr description is different now |
PR Reviewer Guide 🔍
|
2 participants
User description
Summary
sendInstrumentationnow passesanalytics.WithConfiguration(eng.GetConfiguration())alongside the existingWithLogger(logger)option toGetV2InstrumentationObject, so real CLI invocations opt into GAF's new shape-based secret scrubbing for analytics extension data.sendInstrumentationchanges.Implements ticket 02 of the
agent-feedback-secret-redactionspec (seecli-extension-axirepo,.scratch/agent-feedback-secret-redaction/).Blocked — do not merge yet
This depends on
analytics.WithConfiguration, added in go-application-framework#704, which is still open. This branch does not build against the currently-pinned GAF version. Once #704 merges and a GAF version containing it is tagged,cliv2/go.modneeds a real dependency bump before this can merge — that bump is intentionally not included here.Verified locally against a temporary
replace github.com/snyk/go-application-framework => ../../go-application-frameworkpointed at #704's branch (not included in this diff): builds clean,go vetclean, tests pass.Test plan
go test ./cliv2/pkg/core/...passes (locally, with the temporary GAF replace)Test_sendInstrumentation_passesEngineConfigurationToInstrumentationObjectassertseng.GetConfiguration()is consulted for the new option via a gomockMockEnginego.modis bumped for realPR Type
Enhancement, Dependencies
Description
Wire
go-application-frameworkconfiguration into analytics.Ensure specific sensitive data is excluded from redaction.
Update
go-application-frameworkto v0.15.0.File Walkthrough
2 files
Pass engine configuration to analytics instrumentation.Implement unconditional redaction term population.2 files
Add test for analytics configuration passing.Add tests for redaction term population logic.4 files
Update go-application-framework dependency.Update go.sum for dependency change.Update go-application-framework dependency.Update go.sum for dependency change.