Skip to content

fix(security): Object.assign on channel_metadata is a Blocking SAST finding on main — blocks every pre-push #879

Description

@scottschreckengaust

mise run security:sast fails on main with one Blocking semgrep finding, so the pre-push git hook rejects every push from every branch until it is fixed:

cdk/src/handlers/linear-webhook-processor.ts
 ❯❱ javascript.lang.security.insecure-object-assign.insecure-object-assign
       ❰❰ Blocking ❱❱
       Depending on the context, user control data in `Object.assign` can cause web response to
       include data that it should not have or can lead to a mass assignment vulnerability.
       Details: https://sg.run/2R0D
       926┆ Object.assign(channelMetadata, vaultMetadata(resolved));

Introduced by #831 (12c9b63f).

Why CI is green while local pushes are blocked

The whole-repo security:sast runs in security.yml (scheduled / main) and in the pre-push hook. security-pr.yml runs only the ranged variants (security:secrets:range, security:sast:masking:range, security:deps, security:gh-actions). So a finding that lands on main never reds a PR but does block every contributor's git push — the asymmetry that makes this worth fixing rather than waiting for it to surface in CI.

Exploitability

Not exploitable today. vaultMetadata returns a freshly-built literal whose two keys are hard-coded (linear_provider_name, linear_vault_user_id), so there is no attacker-controlled key to smuggle a __proto__ through. The finding is about the capability, which is real: Object.assign copies via [[Set]], which invokes the __proto__ setter, so the pattern is one refactor away from being a prototype-pollution sink.

Proposed fix

Use object spread, matching the four sibling channel_metadata builders (lines 2223, 2448, 2617, 2747):

channelMetadata = { ...channelMetadata, ...vaultMetadata(resolved) };

Spread uses CreateDataProperty (define, not set), so __proto__ from an untrusted source would become an ordinary own property instead of mutating the prototype — the capability is removed, not relocated.

Deliberately not "explicit keyed writes". vaultMetadata's contract (line 638–650) is that every builder spreads it; restating the field list at this one site is the exact bug the helper exists to prevent — add a third vault field later and this path silently drops it, handing the agent no provider on a vault-managed workspace.

Bonus: the source-level guard in cdk/test/handlers/linear-webhook-processor.test.ts keys off the literal-spread form, which is why (per the comment at lines 917–921) the one path it was written for was the one path it never covered. Converting to spread brings this site under that guard.

Scope

  • cdk/src/handlers/linear-webhook-processor.ts — one statement; const channelMetadata becomes let because it is now reassigned.
  • Regression test asserting the vault fields still reach channel_metadata on the vault-onboarded path, plus the source-level guard now covering this builder.
  • No behaviour change: the two keys written are identical.

Also noticed (not in scope)

Line 925 (channelMetadata.linear_workspace_id = workspaceId;) is a redundant re-write — line 861 already sets that key to the same value in the declaring literal. Harmless; flagging rather than folding it in to keep the security fix reviewable on its own.

Activity

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

Metadata

Metadata

Labels

approvedWhen an issue has been approved and readyv1Version 1

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions