aws-samples / aws-samples/sample-autonomous-cloud-coding-agents

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

Open
#879 1 comment 0 reactions 1 assignee Claimed by @scottschreckengaust View on GitHub
approved
Dominant language
TypeScript
Stars
143
Forks
46
Avg merge
3d 9h
Merged PRs (30d)
20

Description

`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):

```ts
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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.