# QSP-38874 — Security Hardening: Fix Security Review Findings

## Summary

Resolved all 6 findings (2 HIGH, 3 MEDIUM, 1 LOW) from the security review of the Dynatrace Scorecards app. All fixes are confined to the API layer and the URL utility — no UI changes.

---

## What Changed

### Findings Fixed

| # | Severity | Finding | File |
|---|---|---|---|
| 1 | HIGH | DQL injection via unvalidated `version` parameter | `scorecard-results.function.ts` |
| 2 | HIGH | 3 unused OAuth scopes + orphaned `@dynatrace-sdk/client-state` package | `app.config.cts`, `package.json` |
| 3 | MEDIUM | Incomplete DQL escaping in `findMatchingScorecardDefinition` | `generate-scorecards-result-event.function.ts` |
| 4 | MEDIUM | Query builders lacked self-contained input validation | `signal-event.ts`, `component-smartscape-node.ts` |
| 5 | MEDIUM | `safeOpenLinkInNewTab` relied on DOMPurify alone for URL safety | `url.utils.ts` |
| 6 | LOW | `signalIds` not DQL-escaped in `array(...)` expression | `signal-event.ts` |

### Files Modified

| File | Change |
|---|---|
| `app.config.cts` | Removed `storage:events:read`, `state:app-states:read`, `state:app-states:write` |
| `package.json` / `package-lock.json` | Removed `@dynatrace-sdk/client-state` (zero imports) |
| `src/api/scorecard-results.function.ts` | Validates `version` — throws `{ status: 400 }` unless `"summary"` or valid SemVer |
| `src/api/generate-scorecards-result-event.function.ts` | 5-step DQL escape: `\` → `"` → `\n` → `\r` → `\|` |
| `src/api/signals/signal-event.ts` | `assetId` + `assetVersion` guards; `signalId` escaping in `array(...)` |
| `src/api/catalog/component-smartscape-node.ts` | `assetId` guard in `buildCatalogComponentQuery` |
| `src/app/utils/url.utils.ts` | DOMPurify (layer 1) + `new URL()` scheme allowlist `https:`/`http:` (layer 2) |

### Tests Modified / Added

| File | Change |
|---|---|
| `src/api/tests/scorecard-results.function.spec.ts` | +9 version validation tests; +5 `buildAppIdLookupQuery` DQL escaping tests (`"`, `\`, `\|`, `\n`, `\r`) |
| `src/api/signals/signal-event.spec.ts` | +5 tests: assetId/assetVersion guards, signalId `"` and `\` escaping |
| `src/api/catalog/component-smartscape-node.spec.ts` | +2 tests: invalid assetId throws, valid passes |
| `src/api/tests/generate-scorecards-result-event.function.spec.ts` | +6 tests: DQL escaping for `\`, `"`, `\n`, `\|`, `undefined`→`""`; invalid assetId |
| `src/app/utils/url.utils.spec.ts` | Removed duplicate `javascript:` test; updated 9 existing → `window.open` NOT called for unsafe URLs |

---

## Commit Message Proposal

```
fix(security): resolve all 6 findings from QSP-38874 security review

- Validate version param in scorecardResults() — reject non-summary/non-SemVer with 400
- Remove 3 unused OAuth scopes and orphaned @dynatrace-sdk/client-state package
- Expand DQL escaping in findMatchingScorecardDefinition (5-step: \, ", \n, \r, |)
- Add assetId/assetVersion defence-in-depth guards in query builders
- Add URL scheme allowlist (https/http) alongside DOMPurify in safeOpenLinkInNewTab
- Escape signalIds in array() expression as defence-in-depth

Closes QSP-38874
```

---

## Validation Results

| Check | Result |
|---|---|
| `npm run typecheck` | ✅ exit 0 |
| `npm run lint` | ✅ 0 errors |
| Unit tests (153 tests) | ✅ 153/153 pass |
| Component tests (94 tests) | ✅ 94/94 pass |
| Coverage — `scorecard-results.function.ts` | ✅ Stmts 97.18% / Branch 92% / Funcs 100% / Lines 96.96% |
| Coverage — `generate-scorecards-result-event.function.ts` | ✅ Stmts 100% / Branch 91.66% / Funcs 100% / Lines 100% |
| Coverage — `signal-event.ts` | ✅ Stmts 100% / Branch 100% / Funcs 100% / Lines 100% |
| Coverage — `component-smartscape-node.ts` | ✅ Stmts 100% / Branch 100% / Funcs 100% / Lines 100% |
| Coverage — `url.utils.ts` | ✅ Stmts 100% / Branch 100% / Funcs 100% / Lines 100% |
| Agent Smith FAST | ✅ PASS |
| Agent Smith FULL | ✅ PASS (Loop 2 — 0 findings) |
| Browser verify | ✅ Confirmed — app still working on `gmg80500.dev.apps.dynatracelabs.com` |


