Skip to content

feat: emit contributor billing after native code test --report - #169

Open
brettgurman-snyk wants to merge 3 commits into
mainfrom
bg/contributor-billing-emit
Open

feat: emit contributor billing after native code test --report#169
brettgurman-snyk wants to merge 3 commits into
mainfrom
bg/contributor-billing-emit

Conversation

@brettgurman-snyk

Copy link
Copy Markdown

Summary

Fire-and-forget AC billing emit for native snyk code test --report: after a successful analyze with a project ID, POST to ES ingest via GAF pkg/contributorbilling.

Legacy TS Code paths are out of scope (cliv2 follow-up).

Changes

  • internal/contributorbilling/ — wrapper around EmitContributorBilling (capability=code, git collection from INPUT_DIRECTORY)
  • native_workflow.go — emit when --report is on, analyze succeeded, and resultMetaData.ProjectId is set

Payload: contributors_entity_id=project:{ProjectId}.

Merge order

  1. GAF IANDT-237 released
  2. This PR (with go.mod GAF bump) — parallel with os-flows / iac extension PRs
  3. cliv2 — bump code-client-go + GAF, WaitWithTimeout, legacy paths
  4. Pre-prod E2E

Test plan

  • go test ./internal/contributorbilling/...
  • Pre-prod: native snyk code test --report → ingest POST observed (after GAF + cliv2 land)

@brettgurman-snyk
brettgurman-snyk requested a review from a team as a code owner July 30, 2026 21:56
@CLAassistant

CLAassistant commented Jul 30, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ fsargent
❌ brettgurman-snyk
You have signed the CLA already but the status is still pending? Let us recheck it.

@snyk-io

snyk-io Bot commented Jul 30, 2026

Copy link
Copy Markdown

⚠️ Snyk checks are incomplete.

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
Secrets 0 0 0 0 0 issues

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

@snyk-io

snyk-io Bot commented Jul 30, 2026

Copy link
Copy Markdown

⚠️ Snyk checks are incomplete.

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.

@snyk-pr-review-bot

This comment has been minimized.

Resolve native_workflow conflicts (keep observability + billing emit),
pin go-application-framework to IANDT-237 pre-release commit, and extract
maybeEmitContributorBilling to satisfy gocyclo lint.

Co-authored-by: Cursor <cursoragent@cursor.com>
@snyk-pr-review-bot

This comment has been minimized.

Signed-off-by: Felix Sargent <felix.sargent@snyk.io>
@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Redundant Error Check 🟡 [minor]

The check analyzeErr != nil in maybeEmitContributorBilling is redundant because the caller EntryPointNative already returns early on non-recoverable errors. Furthermore, if analyzeFnc returns both a valid resultMetaData and a non-nil error (e.g., partial success or warnings), this check will prevent billing emission for a scan that actually succeeded in creating a project record.

if analyzeErr != nil || resultMetaData == nil || resultMetaData.ProjectId == "" {
Possible 401 on Auth Mismatch 🟡 [minor]

The authHeader function prioritizes AUTHENTICATION_TOKEN over AUTHENTICATION_BEARER_TOKEN. If a user has a legacy or environment-provided static token configured alongside a short-lived Bearer token (common in CI/CD environments or IDEs), the billing emission will attempt to use the static token, potentially leading to 401 Unauthorized errors at the ingest endpoint if the static token is no longer valid.

if token := strings.TrimSpace(config.GetString(configuration.AUTHENTICATION_TOKEN)); token != "" {
	return "token " + token
}
if bearer := strings.TrimSpace(config.GetString(configuration.AUTHENTICATION_BEARER_TOKEN)); bearer != "" {
	return "Bearer " + bearer
}
📚 Repository Context Analyzed

This review considered 9 relevant code sections from 7 files (average relevance: 0.92)

🤖 Repository instructions applied (from AGENTS.md)

Timeout: contributorbilling.DefaultTimeout,
Logger: ictx.GetEnhancedLogger(),
Items: []contributorbilling.BillingItem{
{EntityID: projectID},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The EntityID is missing the project: prefix. According to the PR description, the payload should be contributors_entity_id=project:{ProjectId}, but the code only passes the bare projectID. This will send incorrect entity identifiers to the billing system.

{EntityID: "project:" + projectID},
Suggested change
{EntityID: projectID},
{EntityID: "project:" + projectID},

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@fsargent

Copy link
Copy Markdown
Contributor

The build fails because go.mod uses GAF v0.11.0, which lacks pkg/contributorbilling. The former pinned commit is
no longer reachable by Go. Current GAF work moved this package under internal.

This needs an upstream decision:

@fsargent
fsargent self-requested a review August 14, 2026 15:35
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