fix: address all open CodeQL security alerts by marcoscaceres · Pull Request #480 · speced/respec-web-services

marcoscaceres · 2026-04-15T01:04:31Z

Fixes all 6 open CodeQL code-scanning alerts and resolves Dependabot alert #7 (html-minifier ReDoS).

Alert	File	Fix
#3 Reflected XSS	`routes/w3c/group.ts`	Set `Content-Type: text/plain` on 404 error response
#20 Reflected XSS	`routes/github/commits.ts`	Set `Content-Type: text/plain` on 404 error response
#8 SSRF	`routes/github/lib/utils/rest.ts`	Validate endpoint URL origin via `tryURL()` helper (not prefix match); same check on pagination URLs
#15 Insecure Helmet	`app.ts`	Dismissed: `frameguard: false` is intentional (service needs iframe embedding by any spec-hosting site for ReSpec pill UI)
#9, #19 Missing rate limiting	`routes/respec/index.ts`	Rate limit GET `/respec/size` via `express-rate-limit` (10 req/min per IP)
#21 Code injection	`static/xref/filter/worker.js`	Dismissed via security dashboard as false positive
Dependabot #7	`pnpm-lock.yaml`	Override unmaintained `html-minifier` with `html-minifier-terser@7` via pnpm overrides

The rate limiter applies only to the unauthenticated GET path. The PUT route is HMAC-authenticated (webhook) and does not need IP-based limiting.

Copilot

Pull request overview

This PR aims to remediate the repository’s open CodeQL security alerts by hardening response headers, constraining outbound GitHub API requests, and adding request rate limiting to the ReSpec size endpoints.

Changes:

Add a sliding-window rate limiter utility and apply it to /respec/size GET/PUT.
Constrain routes/github/lib/utils/rest.ts to only fetch https://api.github.com/ URLs (including pagination URLs).
Adjust security-related response headers (CSP frame-ancestors, Content-Type: text/plain on certain error responses) and add tests/helpers to support the new behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
utils/rate-limit.ts	Introduces sliding-window rate limiting middleware.
routes/respec/index.ts	Applies the new rate limiter to `/respec/size` routes.
routes/github/lib/utils/rest.ts	Adds allowlist validation for GitHub REST endpoints and pagination URLs.
routes/w3c/group.ts	Sets `Content-Type: text/plain` for invalid group-type 404 response.
routes/github/commits.ts	Sets `Content-Type: text/plain` for error responses to mitigate reflected XSS findings.
app.ts	Replaces `frameguard: false` with CSP `frame-ancestors` directive.
static/xref/filter/worker.js	Adds an inline comment on `new Function()` use (intended as suppression).
tests/utils/rate-limit.test.js	Adds unit tests for the new rate limiting middleware.
tests/routes/github/lib/utils/rest.test.js	Adds tests for the new GitHub API endpoint allowlist validation.
tests/jasmine.json	Registers a Jasmine helper to set env vars for tests.
tests/helpers/env.js	Sets env vars required by some modules at import/load time during tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

.github/workflows/test.yml:24

The workflow allows the TypeScript build step to fail (continue-on-error: true), which means CI can go green even if new type errors are introduced (as long as tests still pass). Consider making the build step required (or at least adding a separate non-blocking step that uploads/logs the tsc errors) so type regressions don't get silently ignored.

        continue-on-error: true
      - run: pnpm test

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

marcoscaceres · 2026-04-15T08:47:41Z

@copilot see @sidvishnoi's comments. Can you evaluate the suggestions from a security perspective and make the fixes?

Copilot · 2026-04-15T08:57:12Z

@copilot see @sidvishnoi's comments. Can you evaluate the suggestions from a security perspective and make the fixes?

Done in dcdfa50 and 65fec1e: tightened /respec/size to 10 req/min, removed real-time waiting from the flaky rate-limit spec by mocking Date.now, and revalidated (tests + CodeQL clean). No UI changes, so no screenshot.

* feat: target Node 24 LTS — bump .nvmrc, add engines field, CI on Node 24, drop node-fetch * refactor: replace node-fetch with native fetch, legacyDirname/Filename with import.meta, fix Express 5 param types * fix: apply XSS and SSRF security fixes from #480 * fix: source CI Node version from .nvmrc Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/5e94febe-bb8b-4d99-86e6-050ffa3f7cb0 Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com> * fix: address review feedback for node version source and commits route typing Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/dde0f0f2-efe1-43fc-b5df-d6bcdf6e4d53 * refactor: use typed Request<Params> in contributors and group routes * chore: apply reviewer suggestions for workflow actions and runtime checks Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/ccaf8248-161f-4e1c-be5b-c38f4c43bf24 * fix: add ImportMeta.main typing for CI TypeScript build Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/ddca0bfa-90d9-4424-a983-bdadec983e7d --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

marcoscaceres · 2026-04-19T21:42:28Z

@copilot rebase?

Copilot · 2026-04-19T21:46:10Z

@copilot rebase?

Done in 7eadff9 — I updated this branch with the latest main and resolved the workflow conflict. Validation is green (pnpm build, pnpm test). No UI changes, so no screenshot.

…board

…ot in PR diff

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/94e5f9c7-db01-42a3-9712-c3baf7a4fc1b

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/b9e8d5e9-33f0-4a95-b00a-d59e89c97cd5

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/94e5f9c7-db01-42a3-9712-c3baf7a4fc1b

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/b9e8d5e9-33f0-4a95-b00a-d59e89c97cd5

…e embedding alone

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/5eda4770-da11-4af8-afed-b3a5f75f5607

- Replace custom rate limiter with express-rate-limit (fixes memory exhaustion and IP spoofing bypass vectors) - Remove rate limit from PUT /respec/size (authenticated webhook route) - Tighten frame-ancestors from wildcard to 'self' + respec.org - Use URL origin comparison for SSRF check instead of prefix match - Add URL confusion test case for SSRF validation

- Rewrite assertGitHubAPIUrl to parse URL explicitly (no throw 0 trick) - Separate error messages for invalid URL vs wrong origin - Guard env helper with ??= to preserve developer's local config - Install express-rate-limit as direct dependency

- Use tryURL() helper in SSRF validation per Sid's suggestion, matching the codebase convention from utils/logging.ts. - Remove CSP frame-ancestors directive: the service intentionally allows embedding by any site for the ReSpec pill UI. - Update test to match unified error message.

Verify the express-rate-limit re-export works and returns middleware when called with valid options.

Tests call the middleware directly with mock req/res objects, verifying the site's rate-limiting behavior independent of which library implements it.

ucontent depends on the unmaintained html-minifier@4 which has a high-severity ReDoS vulnerability (dependabot alert #7). Override with the maintained fork html-minifier-terser@7 via pnpm overrides. API-compatible, no code changes needed.

Helmet's default CSP includes frame-ancestors: 'self' which blocks cross-origin iframe embedding. Delete the directive from defaults so the ReSpec pill UI can be embedded on any spec-hosting site.

Add protocol check to assertGitHubAPIUrl so blob: URLs (whose origin matches api.github.com) are rejected. Add positive test for valid URL, pagination URL validation test, and blob:/data: scheme rejection tests.

- Extract tryURL to utils/misc.ts as shared utility (Sid: "can import existing tryURL") - Change env.js helper from ??= to ||= so empty-string env vars get defaults (Sid) - Fix error message wording: "endpoint origin must be" → "expected" (Sid: "origin includes protocol")

Node 22+ provides URL.parse() which returns null on invalid input instead of throwing — exactly what tryURL did. Remove the custom helper from both utils/misc.ts and utils/logging.ts.

Delete utils/rate-limit.ts (was just a re-export). Import express-rate-limit directly so CodeQL can trace the rate limiter.

Copilot

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-advanced-security AI found potential problems Apr 15, 2026

View reviewed changes

Comment thread routes/respec/index.ts Fixed

Comment thread static/xref/filter/worker.js Fixed

github-advanced-security AI found potential problems Apr 15, 2026

View reviewed changes

Comment thread static/xref/filter/worker.js Fixed

marcoscaceres requested a review from Copilot April 15, 2026 01:20

Copilot started reviewing on behalf of marcoscaceres April 15, 2026 01:20 View session

github-advanced-security AI found potential problems Apr 15, 2026

View reviewed changes

Comment thread static/xref/filter/worker.js Fixed

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread static/xref/filter/worker.js Outdated

Comment thread utils/rate-limit.ts Outdated

Comment thread routes/respec/index.ts Outdated

marcoscaceres requested a review from Copilot April 15, 2026 01:31

Copilot started reviewing on behalf of marcoscaceres April 15, 2026 01:31 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread utils/rate-limit.ts Outdated

Comment thread tests/helpers/env.js Outdated

Copilot started work on behalf of marcoscaceres April 15, 2026 01:40 View session

Copilot finished work on behalf of marcoscaceres April 15, 2026 01:45

marcoscaceres requested a review from Copilot April 15, 2026 01:46

Copilot started reviewing on behalf of marcoscaceres April 15, 2026 01:46 View session

Copilot finished work on behalf of marcoscaceres April 15, 2026 01:47

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread app.ts Outdated

Comment thread .github/workflows/test.yml Outdated

Comment thread .github/workflows/test.yml Outdated

Comment thread tests/utils/rate-limit.test.js Outdated

marcoscaceres mentioned this pull request Apr 15, 2026

feat: migrate to Node 24 LTS #481

Merged

Copilot started work on behalf of marcoscaceres April 15, 2026 02:13 View session

Copilot finished work on behalf of marcoscaceres April 15, 2026 02:15

marcoscaceres added a commit that referenced this pull request Apr 15, 2026

fix: apply XSS and SSRF security fixes from #480

7f5c397

marcoscaceres requested a review from sidvishnoi April 15, 2026 03:05

sidvishnoi reviewed Apr 15, 2026

View reviewed changes

Comment thread utils/rate-limit.ts Outdated

Comment thread tests/utils/rate-limit.test.js Outdated

Comment thread .github/workflows/test.yml

Comment thread routes/respec/index.ts Outdated

Copilot started work on behalf of marcoscaceres April 15, 2026 08:47 View session

Copilot finished work on behalf of marcoscaceres April 15, 2026 08:59

Copilot started work on behalf of marcoscaceres April 19, 2026 21:42 View session

marcoscaceres and others added 20 commits April 25, 2026 15:16

chore: simplify worker.js comment; alerts dismissed via security dash…

883d604

…board

fix(rate-limit): throw RangeError for invalid windowMs or max options

ed58a57

revert: stop touching worker.js; alert #21 dismissed via dashboard, n…

d17e2de

…ot in PR diff

fix(rate-limit): compute sliding-window Retry-After precisely

5de4946

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/94e5f9c7-db01-42a3-9712-c3baf7a4fc1b

test: use os.tmpdir for DATA_DIR in env helper

655c5eb

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/b9e8d5e9-33f0-4a95-b00a-d59e89c97cd5

test(rate-limit): stabilize Retry-After test spy usage

a76fda1

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/94e5f9c7-db01-42a3-9712-c3baf7a4fc1b

test: restore Date.now spy cleanup in rate-limit specs

086b78e

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/b9e8d5e9-33f0-4a95-b00a-d59e89c97cd5

test: make Retry-After spy values fully deterministic

459944c

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/b9e8d5e9-33f0-4a95-b00a-d59e89c97cd5

fix(helmet): disable frameguard so CSP frame-ancestors controls ifram…

32c96aa

…e embedding alone

fix: tighten respec rate limit and remove realtime wait in test

1144d0c

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/5eda4770-da11-4af8-afed-b3a5f75f5607

fix: keep precise Retry-After math and clarify mocked expiry timestamp

78aa893

Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/5eda4770-da11-4af8-afed-b3a5f75f5607

fix: address Copilot review findings

fcaea5a

- Rewrite assertGitHubAPIUrl to parse URL explicitly (no throw 0 trick) - Separate error messages for invalid URL vs wrong origin - Guard env helper with ??= to preserve developer's local config - Install express-rate-limit as direct dependency

test: add smoke tests for rate-limit re-export

07977a4

Verify the express-rate-limit re-export works and returns middleware when called with valid options.

test: add behavior tests for rate limiting middleware

9c5c53f

Tests call the middleware directly with mock req/res objects, verifying the site's rate-limiting behavior independent of which library implements it.

fix(helmet): remove frame-ancestors from default CSP

087e94b

Helmet's default CSP includes frame-ancestors: 'self' which blocks cross-origin iframe embedding. Delete the directive from defaults so the ReSpec pill UI can be embedded on any spec-hosting site.

test: assert Retry-After header on 429 response

ed82db5

fix: reject non-https schemes in SSRF check, expand tests

4e12de1

Add protocol check to assertGitHubAPIUrl so blob: URLs (whose origin matches api.github.com) are rejected. Add positive test for valid URL, pagination URL validation test, and blob:/data: scheme rejection tests.

marcoscaceres force-pushed the fix/codeql-security-alerts branch from 6471d07 to 4e12de1 Compare April 25, 2026 05:22

marcoscaceres added 3 commits April 25, 2026 17:18

chore: regenerate lockfile after dep-updates merge

02cebe0

refactor: replace tryURL with native URL.parse()

44be310

Node 22+ provides URL.parse() which returns null on invalid input instead of throwing — exactly what tryURL did. Remove the custom helper from both utils/misc.ts and utils/logging.ts.

marcoscaceres requested a review from Copilot April 25, 2026 09:40

Copilot started reviewing on behalf of marcoscaceres April 25, 2026 09:40 View session

refactor: inline express-rate-limit import, remove wrapper

fe7452a

Delete utils/rate-limit.ts (was just a re-export). Import express-rate-limit directly so CodeQL can trace the rate limiter.

Copilot AI reviewed Apr 25, 2026

View reviewed changes

marcoscaceres merged commit 21ee6e1 into main Apr 25, 2026
3 checks passed

marcoscaceres deleted the fix/codeql-security-alerts branch April 25, 2026 10:24

Uh oh!

Conversation

marcoscaceres commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marcoscaceres commented Apr 15, 2026

Uh oh!

Copilot AI commented Apr 15, 2026

Uh oh!

marcoscaceres commented Apr 19, 2026

Uh oh!

Copilot AI commented Apr 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

marcoscaceres commented Apr 15, 2026 •

edited

Loading