Skip to content

Merge pull request #6864 from snyk/docs/agents.md#6867

Open
j-luong wants to merge 2 commits into
mainfrom
chore/cli-1372_addOrgLookupInstrumentationData
Open

Merge pull request #6864 from snyk/docs/agents.md#6867
j-luong wants to merge 2 commits into
mainfrom
chore/cli-1372_addOrgLookupInstrumentationData

Conversation

@j-luong
Copy link
Copy Markdown
Contributor

@j-luong j-luong commented Jun 1, 2026

docs: adding basic agents.md

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Commit messages
    are release-note ready, emphasizing
    what was changed, not how.
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)
  • Includes product update to be announced in the next stable release notes

What does this PR do?

This PR adds some new instrumentation data to measure how we perform organisation lookups in GAF.

Where should the reviewer start?

Related GAF PR

How should this be manually tested?

Run the following commands and check for the instrumentation entry:

command: `snyk test --org=86616ead-043b-4ef3-8c8d-81ecdf5ac751 -d` // valid UUID but org does not exist
instrumentation: "gaf.app.defaultfunc.organization.lookup": "provided_id" // should be "provided_id_failed"

command: `snyk test --org=be9221c6-5f63-4339-a07e-43d38b3ecb8z -d` // this is not actually a valid UUID and will be considered a slugname in code
instrumentation: "gaf.app.defaultfunc.organization.lookup": "provided_slug_failed"

command: `snyk test --org=<VALID_ORG_ID> -d` // valid UUID and org exists
instrumentation: "gaf.app.defaultfunc.organization.lookup": "provided_id"

command: `snyk test --org=<VALID_ORG_SLUG> -d`
instrumentation: "gaf.app.defaultfunc.organization.lookup": "provided_slug"

command: `snyk test --org=fake -d`
instrumentation: "gaf.app.defaultfunc.organization.lookup": "provided_slug_failed"

command: `snyk test -d`
instrumentation: "gaf.app.defaultfunc.organization.lookup": "default"

Note that:

command: `snyk test --org=86616ead-043b-4ef3-8c8d-81ecdf5ac751 -d` // valid UUID but org does not exist
instrumentation: "gaf.app.defaultfunc.organization.lookup": "provided_id" // should be "provided_id_failed"

Outputs the wrong instrumentation data as outlined in RFC: Consistent Organization Resolution.

What's the product update that needs to be communicated to CLI users?

N/A - internal change

Risk assessment (Low | Medium | High)?

Low - changes to instrumentation data

@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented Jun 1, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Comment thread cliv2/pkg/core/main.go

// We want to scrub the debug log of sensitive information. Since we have a list of commands we know can occur, we can intersect that with arguments we don't recognize, and automatically scrub all those from the logs.
if debugEnabled {
writeLogHeader(globalConfiguration, networkAccess)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context: writeLogHeader will call config.GetString(configuration.ORGANIZATION) before analytics are initialised, so we move it to just after globalEngine.Init()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the goal is to call config.GetString(configuration.ORGANIZATION), maybe we should do it explicitly here? wdyt?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its right where it currently is, config.GetString(configuration.ORGANIZATION) fetches the configured org so should be called close to where it is going to be used - which is in writeLogHeader()

@j-luong j-luong marked this pull request as ready for review June 1, 2026 11:08
@j-luong j-luong requested a review from a team as a code owner June 1, 2026 11:08
@snyk-pr-review-bot

This comment has been minimized.

chore: add scripts for metadata and shasum validation
@j-luong j-luong force-pushed the chore/cli-1372_addOrgLookupInstrumentationData branch from 5f49a4b to 3a47883 Compare June 4, 2026 12:49
@snyk-pr-review-bot

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

Warnings
⚠️ There are multiple commits on your branch, please squash them locally before merging!

Generated by 🚫 dangerJS against bd8fe93

@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
The change in cliv2/pkg/core/main.go explicitly adds the user's Organization ID and Organization Slug to a list of terms that are exempted from the log scrubbing process. This will cause sensitive customer identifiers to leak into debug logs whenever the CLI is run with --debug or the DEBUG environment variable.

⚡ Recommended focus areas for review

Log Context Loss 🟡 [minor]

Moving writeLogHeader to after globalEngine.Init() creates a gap in log visibility. If globalEngine.Init() fails, it returns an error and the function exits at line 622. In this scenario, any logs generated during initialization or the failure itself will be missing the diagnostic header information (version, environment, etc.), making troubleshooting harder.

writeLogHeader(globalConfiguration, networkAccess)
📚 Repository Context Analyzed

This review considered 8 relevant code sections from 5 files (average relevance: 0.94)

🤖 Repository instructions applied (from AGENTS.md)

@robertolopezlopez
Copy link
Copy Markdown
Contributor

I think the PR description (docs: adding basic agents.md) does not correspond with the diff @j-luong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants