Skip to content

feat: support Snyk Code Local Engine on the native code-client-go path - #158

Merged
graphite-app[bot] merged 1 commit into
mainfrom
coin-2539-scle-native-parity
Jun 24, 2026
Merged

feat: support Snyk Code Local Engine on the native code-client-go path#158
graphite-app[bot] merged 1 commit into
mainfrom
coin-2539-scle-native-parity

Conversation

@fsargent

@fsargent fsargent commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Description

Enables the native (code-client-go) Snyk Code path for Snyk Code Local Engine (SCLE) orgs, and makes OAuth2 auth work against the local engine — the functional-parity prerequisite for removing the TypeScript @snyk/code-client dependency from the CLI (COIN-2539, CLI-589).

SCLE was disabled on the native path via a && !scleEnabled guard. Three things were missing to support it; this PR addresses all three:

  1. URL mappingSnykCodeApi() now returns localCodeEngine.url from the cached SAST settings when SCLE is enabled, instead of the naive apideeproxy string replacement that ignored it (new localCodeEngineURL() helper).
  2. Auth — register the local engine URL in AUTHENTICATION_ADDITIONAL_URLS. GAF's networking middleware (middleware/auth_header.go) only attaches credentials to the configured API host, its deeproxy subdomain, or URLs in that allowlist; a customer-hosted SCLE URL matches none, so without this the engine is called unauthenticated. Registering it applies the standard auth flow (incl. OAuth2 bearer + refresh) to SCLE requests.
  3. API surface (per @peter.schafer's review) — SCLE implements the old deeproxy API, not the new test service that UploadAndAnalyzeRunTest uses. SCLE scans are now routed through the legacy scanner (UploadAndAnalyzeLegacyRunLegacyTest, the deeproxy implementation the IDE team moved into this repo), draining its status channel for progress.

Plus: drop the && !scleEnabled guard, and centralize the internal_sast_settings / internal_snyk_scle_enabled config keys in code_workflow (aliased from pkg/code).

Decisions (from review)

  • --report on SCLE is not supported (reporting goes through the test service). It now errors early with a clear message instead of silently using the test-service path. Covered by Test_defaultAnalyzeFunction_reportNotSupportedWithSCLE.
  • shardKey is passed empty — deeproxy uses it only for cache sharding, so it is optional for a CLI scan (the IDE supplies one for editor caching). Documented at the call site.

Tests

Test_SnykCodeApi, Test_registerLocalEngineAuthURL, updated useNativeImplementation + Test_Code_entrypoint. The SCLE→legacy-scanner selection is not unit-covered (the scanner isn't injectable into defaultAnalyzeFunction) — it needs the live/e2e run below.

Still required before broad enablement: an end-to-end run against the scle-prod-cluster org (Code Integrations owns it) with {OAuth2, token} — the unit tests cover URL/auth/routing logic, not a real local-engine handshake.

Checklist

  • Tests added and all succeed
  • Linted
  • README.md updated, if user-facing — n/a (internal routing change, no public API change)

🚨After having merged, please update the snyk-ls and CLI go.mod to pull in latest client.

Refs: COIN-2539, CLI-589

@snyk-io

snyk-io Bot commented Jun 16, 2026

Copy link
Copy Markdown

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.

Copy link
Copy Markdown
Contributor Author

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@snyk-io

snyk-io Bot commented Jun 16, 2026

Copy link
Copy Markdown

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.

@fsargent
fsargent marked this pull request as ready for review June 16, 2026 11:01
@fsargent
fsargent requested a review from a team as a code owner June 16, 2026 11:01
@github-actions github-actions Bot added feature and removed feature labels Jun 16, 2026
@snyk-pr-review-bot

This comment has been minimized.

@fsargent
fsargent force-pushed the coin-2539-scle-native-parity branch from f5f0863 to 10fb8e2 Compare June 16, 2026 11:03
@github-actions github-actions Bot added feature and removed feature labels Jun 16, 2026
@snyk-pr-review-bot

This comment has been minimized.

@github-actions github-actions Bot removed the feature label Jun 16, 2026
@fsargent
fsargent force-pushed the coin-2539-scle-native-parity branch from 10fb8e2 to c2ef599 Compare June 16, 2026 11:21
@github-actions github-actions Bot added feature and removed feature labels Jun 16, 2026
@snyk-pr-review-bot

This comment has been minimized.

@github-actions github-actions Bot removed the feature label Jun 16, 2026
@fsargent
fsargent force-pushed the coin-2539-scle-native-parity branch from c2ef599 to a54a99b Compare June 16, 2026 12:08
@github-actions github-actions Bot added feature and removed feature labels Jun 16, 2026
@fsargent

fsargent commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@PeterSchafer — you're right, that was the missing piece. Confirmed in the code: defaultAnalyzeFunction was calling UploadAndAnalyzeRunTest (the new test-service API), which SCLE doesn't implement.

Pushed a commit that routes SCLE scans through the legacy/deeproxy scanner the IDE team moved in (UploadAndAnalyzeLegacyRunLegacyTest), in addition to the URL + auth mapping. So all three pieces are now in place: SCLE URL → localCodeEngine.url, auth via AUTHENTICATION_ADDITIONAL_URLS, and the deeproxy API surface.

Two things I'd value your read on (also in the description):

  1. --report on SCLERunLegacyTest has no test-service reporting, and the remoteCode branch still returns early via AnalyzeRemote before the SCLE check. Should snyk code test --report work on SCLE, or should we error?
    I think we just error, right?
  2. shardKey — passing empty for now (deeproxy cache sharding only); the IDE supplies one. OK for the CLI?

@snyk-pr-review-bot

This comment has been minimized.

@fsargent
fsargent force-pushed the coin-2539-scle-native-parity branch from a54a99b to 6befdbd Compare June 16, 2026 14:43
@fsargent

fsargent commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Addressed review:

  • API surface: SCLE now routes through UploadAndAnalyzeLegacyRunLegacyTest (deeproxy), not the test service.
  • --report on SCLE: now errors early ("--report is not supported with Snyk Code Local Engine") rather than silently hitting the test-service path. Added a unit test.
  • shardKey: passing empty — deeproxy uses it only for cache sharding, optional for a CLI scan. Documented at the call site.

Automated-review findings:

  • Data loss in ResultMetaData (major) — this is a false positive: UploadAndAnalyzeLegacy returns no ResultMetaData (web UI URL / project / snapshot id are test-service concepts that don't exist for an SCLE file test), and TranslateMetadataToLocalFindingModel is fully nil-guarded. There's no metadata to lose.
  • Inconsistent signature (minor) — fixed: analyzeWithLegacyEngine now returns the 4-tuple matching OptionalAnalysisFunctions, with an explicit nil ResultMetaData + comment explaining why.

Still pending: live run against scle-prod-cluster to confirm the end-to-end handshake.

@github-actions github-actions Bot added feature and removed feature labels Jun 16, 2026
@snyk-pr-review-bot

This comment has been minimized.

Comment thread internal/commands/code_workflow/code_client_helper.go
Comment thread internal/commands/code_workflow/code_client_helper.go Outdated
Comment thread internal/commands/code_workflow/native_workflow.go
Comment thread internal/commands/code_workflow/native_workflow.go
Comment thread internal/commands/code_workflow/native_workflow_test.go
Comment thread pkg/code/code.go Outdated

@danskmt danskmt left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM - left some comments

Comment thread internal/commands/code_workflow/native_workflow.go
@fsargent
fsargent force-pushed the coin-2539-scle-native-parity branch from 6befdbd to 44f5d4c Compare June 22, 2026 19:44
@snyk-pr-review-bot

This comment has been minimized.

@fsargent
fsargent force-pushed the coin-2539-scle-native-parity branch from 44f5d4c to 5f73740 Compare June 24, 2026 11:38
@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

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

Runtime Panic 🔴 [critical]

The function analyzeWithLegacyEngine explicitly returns nil for *scan.ResultMetaData. However, the caller EntryPointNative (visible in context) passes this metadata directly to local_models.TranslateMetadataToLocalFindingModel(resultMetaData, ...). Since that function attempts to read fields from the metadata to populate the local model, passing nil will cause a runtime panic during every SCLE scan.

return result, bundleHash, nil, err
Configuration Inconsistency 🟡 [minor]

The SnykCodeApi function returns an empty string if tryGetLocalCodeEngineURL fails while SCLE is enabled. This results in the codeclient being initialized with an empty base URL, which will lead to malformed requests (e.g., requests to path /bundle instead of https://engine/bundle). While defaultAnalyzeFunction adds a check, other potential callers of SnykCodeApi or direct library users of codeClientConfig are not protected by that check.

return ""
📚 Repository Context Analyzed

This review considered 23 relevant code sections from 13 files (average relevance: 0.97)

🤖 Repository instructions applied (from AGENTS.md)

// Reporting goes through the test service, which Snyk Code Local Engine does
// not implement, so --report is not supported for SCLE.
if reportMode != noReport && config.GetBool(ConfigurationSlceEnabled) {
return nil, "", nil, errors.New("--report is not supported with Snyk Code Local Engine")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we use an explicit Error Catalog error to avoid a generic CLI failure like Peter suggested?

@graphite-app

graphite-app Bot commented Jun 24, 2026

Copy link
Copy Markdown

Merge activity

#158)

### Description

Enables the native (`code-client-go`) Snyk Code path for **Snyk Code Local Engine (SCLE)** orgs, and makes OAuth2 auth work against the local engine — the functional-parity prerequisite for removing the TypeScript `@snyk/code-client` dependency from the CLI ([COIN-2539](https://snyksec.atlassian.net/browse/COIN-2539), [CLI-589](https://snyksec.atlassian.net/browse/CLI-589)).

SCLE was disabled on the native path via a `&& !scleEnabled` guard. Three things were missing to support it; this PR addresses all three:

1. **URL mapping** — `SnykCodeApi()` now returns `localCodeEngine.url` from the cached SAST settings when SCLE is enabled, instead of the naive `api`→`deeproxy` string replacement that ignored it (new `localCodeEngineURL()` helper).
2. **Auth** — register the local engine URL in `AUTHENTICATION_ADDITIONAL_URLS`. GAF's networking middleware (`middleware/auth_header.go`) only attaches credentials to the configured API host, its `deeproxy` subdomain, or URLs in that allowlist; a customer-hosted SCLE URL matches none, so without this the engine is called **unauthenticated**. Registering it applies the standard auth flow (incl. OAuth2 bearer + refresh) to SCLE requests.
3. **API surface** (per @peter.schafer's review) — SCLE implements the **old deeproxy API**, not the **new test service** that `UploadAndAnalyze` → `RunTest` uses. SCLE scans are now routed through the legacy scanner (`UploadAndAnalyzeLegacy` → `RunLegacyTest`, the deeproxy implementation the IDE team moved into this repo), draining its status channel for progress.

Plus: drop the `&& !scleEnabled` guard, and centralize the `internal_sast_settings` / `internal_snyk_scle_enabled` config keys in `code_workflow` (aliased from `pkg/code`).

### Decisions (from review)

- **`--report` on SCLE** is **not supported** (reporting goes through the test service). It now **errors early** with a clear message instead of silently using the test-service path. Covered by `Test_defaultAnalyzeFunction_reportNotSupportedWithSCLE`.
- **`shardKey`** is passed **empty** — deeproxy uses it only for cache sharding, so it is optional for a CLI scan (the IDE supplies one for editor caching). Documented at the call site.

### Tests

`Test_SnykCodeApi`, `Test_registerLocalEngineAuthURL`, updated `useNativeImplementation` + `Test_Code_entrypoint`. The SCLE→legacy-scanner *selection* is not unit-covered (the scanner isn't injectable into `defaultAnalyzeFunction`) — it needs the live/e2e run below.

**Still required before broad enablement:** an end-to-end run against the `scle-prod-cluster` org (Code Integrations owns it) with {OAuth2, token} — the unit tests cover URL/auth/routing logic, not a real local-engine handshake.

### Checklist

- [x] Tests added and all succeed
- [x] Linted
- [ ] README.md updated, if user-facing — n/a (internal routing change, no public API change)

🚨After having merged, please update the `snyk-ls` and CLI go.mod to pull in latest client.

Refs: [COIN-2539](https://snyksec.atlassian.net/browse/COIN-2539), [CLI-589](https://snyksec.atlassian.net/browse/CLI-589)

[COIN-2539]: https://snyksec.atlassian.net/browse/COIN-2539?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[CLI-589]: https://snyksec.atlassian.net/browse/CLI-589?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[COIN-2539]: https://snyksec.atlassian.net/browse/COIN-2539?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
@graphite-app
graphite-app Bot force-pushed the coin-2539-scle-native-parity branch from 5f73740 to 650ae39 Compare June 24, 2026 12:03
@graphite-app
graphite-app Bot merged commit 650ae39 into main Jun 24, 2026
14 of 16 checks passed
@graphite-app
graphite-app Bot deleted the coin-2539-scle-native-parity branch June 24, 2026 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants