Skip to content

[APS-19076][APS-19077][APS-19078] fix: security hardening — env-var allowlist, workflow SHA pinning, report HTML sanitization#85

Open
Jimesh-browserstack wants to merge 3 commits intomasterfrom
fix/aps-19076-19077-19078-security-bundle
Open

[APS-19076][APS-19077][APS-19078] fix: security hardening — env-var allowlist, workflow SHA pinning, report HTML sanitization#85
Jimesh-browserstack wants to merge 3 commits intomasterfrom
fix/aps-19076-19077-19078-security-bundle

Conversation

@Jimesh-browserstack
Copy link
Copy Markdown

@Jimesh-browserstack Jimesh-browserstack commented May 7, 2026

Security Bundle: APS-19076 / APS-19077 / APS-19078

This PR resolves three security findings on browserstack/github-actions in a single bundled change. All three target distinct files in the same repo and were approved as a single PR by the assignee.

Ticket Severity Component
APS-19076 Critical 9.3 setup-env env-var injection
APS-19077 High 8.7 CI workflow supply chain + token scope
APS-19078 Medium 7.6 browserstack-report-action stored XSS

APS-19076 — env-var injection from rerun API (CVSS 9.3)

Issue. setBStackRerunEnvVars() in setup-env/src/actionInput/index.js did Object.keys(variables).forEach((k) => core.exportVariable(k, variables[k])) over the BrowserStack rerun API response with no allowlist. A caller who could influence the API response (compromised account, MITM, malicious server-side state) could inject arbitrary env vars into the workflow runner — NODE_OPTIONS=--require /tmp/evil.js, PATH=/tmp/evil:/usr/bin, GITHUB_TOKEN=... overrides — leading to RCE / token exfiltration.

Fix.

  • Added ALLOWED_RERUN_ENV_VARS = ['BROWSERSTACK_RERUN', 'BROWSERSTACK_RERUN_TESTS', 'BROWSERSTACK_BUILD_NAME'] to setup-env/config/constants.js.
  • Filtered the loop: core.exportVariable is now called only for allowlisted keys; everything else triggers core.warning(...) so non-allowlisted names surface in the runner log without silently failing.
  • Rebuilt setup-env/dist/index.js (committed bundle is what the action runs).

Reviewer note. If the rerun API legitimately sets additional names in production, the list will need to grow — they will be visible as core.warning lines in workflow logs. Conservative by design.


APS-19077 — workflow supply chain + token scope (CVSS 8.7)

Issue. .github/workflows/setup-env.yml and setup-local.yml used actions/setup-node@master (floating tag — anyone with push access to setup-node could inject malicious code into our CI) and had no permissions: block (default GITHUB_TOKEN had write scope it didn't need).

Fix.

  • Pinned actions/checkout@v4 -> actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2.
  • Pinned actions/setup-node@master -> actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0.
  • Added top-level permissions: { contents: read } to both files (these workflows only run unit tests; they do not push, comment, or release).

APS-19078 — report HTML stored XSS (CVSS 7.6)

Issue. browserstack-report-action's ReportProcessor.processReport() wrote basicHtml / richHtml / richCss from the BrowserStack reporting backend directly into summary.addRaw(...) (renders in the GitHub Actions summary UI) and the artifact HTML file, with only smart-quote and <tbody> transforms — no sanitization. A malicious build name, test title, or test output could embed <script> / onerror / javascript: URLs and execute JS in the GitHub UI context (CSRF the user, exfiltrate session cookies).

Fix.

  • Added sanitize-html ^2.17.3 to browserstack-report-action/package.json + lockfile.
  • ReportProcessor.js: defined HTML_SANITIZE_OPTIONS (allowed tags/attributes; disallowedTagsMode: 'discard'; no protocol-relative; only http/https/data/mailto schemes); wrapped both basicHtml and richHtml with sanitizeHtml(...). Defined sanitizeCss(css) that strips expression(...), url(javascript:...) and any <script|iframe|object|embed> tags from richCss.
  • UploadFileForArtifact.js: defined CSP meta default-src 'none'; style-src 'unsafe-inline'; img-src 'self' data: https:; font-src data:; script-src 'none'; injectCspMeta(html) inserts it into <head> (or constructs a head if missing) before fs.writeFileSync. Defense-in-depth on top of the sanitizer.
  • Rebuilt browserstack-report-action/dist/index.js.

Testing

Subdir Branch Result
setup-env/ master baseline 36 passing / 0 failing
setup-env/ this PR 37 passing / 0 failing
browserstack-report-action/ master baseline 18 passing / 0 failing
browserstack-report-action/ this PR 18 passing / 0 failing (lint clean)

setup-env: rewrote pre-existing test Sets environment variables from BrowserStack API response (was using VAR1/VAR2 which would now be rejected) to use the three allowlisted names; added negative-case test Rejects non-allowlisted env vars from BrowserStack API response (APS-19076) that asserts NODE_OPTIONS / PATH / GITHUB_TOKEN are NOT exported and that core.warning is called for each.

Manual XSS strip sanity — exercised the same HTML_SANITIZE_OPTIONS used by ReportProcessor.js against six payloads:

Payload Output Safe
<img src=x onerror=alert(1)> <img src="x" /> yes
<script>alert(1)</script> (empty) yes
<a href="javascript:alert(1)">click</a> <a>click</a> yes
<svg onload=alert(1)></svg> (empty) yes
<iframe src="javascript:alert(1)"></iframe> (empty) yes
<p>safe <b>content</b></p> preserved yes (control)

No BrowserStack live session run — this is a GitHub Actions library, not a session/SDK runtime; the actions only set env vars and start the BrowserStackLocal binary, so the bs-session skill does not apply.


Integration test evidence

Both fixes were exercised end-to-end against the built dist/index.js bundles (not just the source) by stubbing the BrowserStack reporting backend on 127.0.0.1 and capturing the action's outputs (GITHUB_ENV, step-summary.md, artifact HTML).

Test Target Result
Test A APS-19076 env-var allowlist 15/15 pass
Test B APS-19078 sanitize + CSP (incl. data:-on-<a> bypass) 25/25 pass

Test A — APS-19076 env-var allowlist (15 assertions, all pass):

  • Script: ~/source/aps-investigations/tickets/APS-19076-19077-19078/repros/aps-19076-test-a-allowlist.js
  • Captured GITHUB_ENV: recorded in ~/source/aps-investigations/tickets/APS-19076-19077-19078/results/APS-19076-test-a-allowlist-20260507-152548.json under githubEnvContents — confirms BROWSERSTACK_RERUN/BROWSERSTACK_RERUN_TESTS/BROWSERSTACK_BUILD_NAME are exported and NODE_OPTIONS/PATH/GITHUB_TOKEN/etc. are dropped with core.warning lines.

Test B — APS-19078 sanitize + CSP (25 assertions, all pass):

  • Script: ~/source/aps-investigations/tickets/APS-19076-19077-19078/repros/aps-19078-test-b-sanitize-csp.js
  • Step-summary output: ~/source/aps-investigations/tickets/APS-19076-19077-19078/results/APS-19078-test-b-summary-20260507-153512.md
  • Artifact HTML output: ~/source/aps-investigations/tickets/APS-19076-19077-19078/results/APS-19078-test-b-artifact-20260507-153512.html
  • Full evidence JSON: ~/source/aps-investigations/tickets/APS-19076-19077-19078/results/APS-19078-test-b-sanitize-csp-20260507-153512.json

data:-URL bypass caught by integration testing — the first dist-level run of Test B failed one assertion: <a href="data:text/html,<script>alert(6)</script>">data-click</a> survived sanitization in both the summary and artifact, because allowedSchemes permitted data: on every element. Folded into this PR as commit 67f66eaallowedSchemes reduced to ['http', 'https', 'mailto'] and allowedSchemesByTag: { img: ['http', 'https', 'data'] } added so legitimate inline <img src="data:image/png;base64,..."> screenshots still render. Test B was extended with a positive assertion that the safe <img data:base64> survives — passes after the fix.


Jira tickets

Checklist

  • Security issues addressed
  • Unit tests passing on both subdirs
  • Lint clean
  • dist bundles rebuilt (setup-env, browserstack-report-action)
  • BrowserStack session run (N/A — not a session repo)
  • No force-push, no skipped hooks, no amend

- APS-19076 (CVSS 9.3, env-var injection): allowlist env-var names
  exported from the BrowserStack rerun API in setBStackRerunEnvVars.
  Adds ALLOWED_RERUN_ENV_VARS (BROWSERSTACK_RERUN, BROWSERSTACK_RERUN_TESTS,
  BROWSERSTACK_BUILD_NAME) to setup-env/config/constants.js; filtered
  loop in setup-env/src/actionInput/index.js calls core.exportVariable
  only for allowlisted keys and core.warning for everything else.
  Updates existing test, adds negative-case test, rebuilds dist.

- APS-19077 (CVSS 8.7, supply chain + token scope): pins
  actions/setup-node@v4.4.0 and actions/checkout@v4.2.2 by SHA in both
  .github/workflows/setup-env.yml and setup-local.yml; adds top-level
  permissions: { contents: read } block to give the GITHUB_TOKEN least
  privilege (these workflows only run unit tests).

- APS-19078 (CVSS 7.6, stored XSS via report HTML): adds sanitize-html
  to browserstack-report-action; sanitizes basicHtml/richHtml in
  ReportProcessor.js with a strict allowlist (no inline event handlers,
  no javascript: URLs); strips JS hooks from richCss with sanitizeCss();
  injects CSP meta (script-src 'none'; default-src 'none') into the
  artifact HTML head in UploadFileForArtifact.js as defense-in-depth;
  rebuilds dist.

Tests: setup-env 37 passing (was 36, +1 negative-case); report-action
18 passing (no regression). Standalone XSS-strip sanity covers six
payloads including <img src=x onerror=alert(1)>, <script>, javascript:
URLs, <svg onload>, <iframe javascript:>; all stripped, safe HTML
preserved.

Resolves: APS-19076, APS-19077, APS-19078

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Jimesh-browserstack Jimesh-browserstack requested a review from a team as a code owner May 7, 2026 14:59
Comment thread browserstack-report-action/src/services/ReportProcessor.js Fixed
Integration test caught a click-to-XSS bypass: <a href="data:text/html,<script>...</script>">
survived because data: was allowed on all elements. Restrict to <img> for inline
screenshots; block on <a>/<iframe>/etc.
CodeQL flagged the `<script|iframe|object|embed>` regex in sanitizeCss as
"incomplete multi-character sanitization" (high severity x2 — src + dist).
A single-pass replace is bypassable by nested patterns like `<scr<script>ipt>`,
which collapse to `<script>` after one substitution.

Fix: strip ALL HTML tags from the rich-CSS payload via sanitize-html with
allowedTags: [] (the library iterates internally and is not bypassable).
The CSS-function regexes for `expression(...)` and `url(javascript:...)`
remain — they target CSS syntax, not HTML, and CodeQL did not flag them.

Verified:
- 18/18 unit tests pass (no regression)
- Sanity script confirms `<scr<script>ipt>` no longer leaves a `<script`
  substring in the output
- Flagged regex literal absent from rebuilt dist/index.js
- Lint clean

Resolves: CodeQL failure on PR #85
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.

2 participants