Skip to content

Commit 21ee6e1

Browse files
fix: address all open CodeQL security alerts (#480)
* fix: replace Helmet frameguard:false with CSP frame-ancestors for iframe embedding * fix: add sliding-window rate limiting to /respec/size routes * chore: suppress intentional code-injection false positive in xref filter worker * test: add tests for rate-limit utility and SSRF URL validation * chore: fix CodeQL suppression syntax; remove redundant comment in app.ts * chore: simplify worker.js comment; alerts dismissed via security dashboard * fix(rate-limit): throw RangeError for invalid windowMs or max options * revert: stop touching worker.js; alert #21 dismissed via dashboard, not in PR diff * fix(rate-limit): compute sliding-window Retry-After precisely 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 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 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 Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/b9e8d5e9-33f0-4a95-b00a-d59e89c97cd5 * test: make Retry-After spy values fully deterministic 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 iframe embedding alone * fix: tighten respec rate limit and remove realtime wait in test 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 Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/5eda4770-da11-4af8-afed-b3a5f75f5607 * fix: use express-rate-limit, tighten CSP and SSRF checks - 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 * fix: address Copilot review findings - 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 * fix: address Sid's review comments - 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. * test: add smoke tests for rate-limit re-export Verify the express-rate-limit re-export works and returns middleware when called with valid options. * test: add behavior tests for rate limiting middleware Tests call the middleware directly with mock req/res objects, verifying the site's rate-limiting behavior independent of which library implements it. * fix: override html-minifier with html-minifier-terser 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. * fix(helmet): remove frame-ancestors from default CSP 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 * fix: reject non-https schemes in SSRF check, expand tests 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. * chore: regenerate lockfile after dep-updates merge * fix: address Sid's review comments on PR #480 - 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") * refactor: replace tryURL with native URL.parse() 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. * refactor: inline express-rate-limit import, remove wrapper Delete utils/rate-limit.ts (was just a re-export). Import express-rate-limit directly so CodeQL can trace the rate limiter. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent dc706bf commit 21ee6e1

10 files changed

Lines changed: 329 additions & 70 deletions

File tree

app.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,15 @@ registerViewEngine(app);
3232

3333
// Security
3434
// Defaults https://www.npmjs.com/package/helmet#how-it-works
35+
// ReSpec pill UI is embedded via iframe on any spec-hosting site,
36+
// so we must not send frame-ancestors or X-Frame-Options.
37+
const cspDirectives = helmet.contentSecurityPolicy.getDefaultDirectives();
38+
delete cspDirectives["frame-ancestors"];
39+
3540
app.use(
3641
helmet({
37-
// Allow for UI inclusion as iframe in ReSpec pill.
3842
frameguard: false,
43+
contentSecurityPolicy: { directives: cspDirectives },
3944
}),
4045
);
4146

package.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"cors": "^2.8.6",
1717
"dotenv": "^17.4.2",
1818
"express": "^5.2.1",
19+
"express-rate-limit": "^8.4.0",
1920
"helmet": "^8.1.0",
2021
"morgan": "^1.10.1",
2122
"nanoid": "^5.1.9",
@@ -34,6 +35,11 @@
3435
"update-data-sources": "node build/scripts/update-data-sources.js"
3536
},
3637
"packageManager": "pnpm@9.8.0",
38+
"pnpm": {
39+
"overrides": {
40+
"html-minifier": "npm:html-minifier-terser@^7.2.0"
41+
}
42+
},
3743
"simple-git-hooks": {
3844
"post-merge": "pnpm i && pnpm build"
3945
},

0 commit comments

Comments
 (0)