Commit 629efd36ed
629efd36edad5b593806dc68dc6a17287c880280
parent: 7b6eae1acc
Unsigned
cmc <hello@cleberg.net> · 2026-07-20 15:16 UTC
Address SonarCloud maintainability findings
Clears the 11 low-severity nits and the medium-severity regex finding.
The high-severity router complexity is left as-is for now.
- base64UrlEncode's `=+$` is replaced with a bounded `={1,2}$`. Base64
padding is never longer than two characters, so the unbounded
quantifier only bought super-linear backtracking (S8786).
- global single-character `replace` calls become `replaceAll` (S7781),
`charCodeAt`/`fromCharCode` become `codePointAt`/`fromCodePoint`
(S7758), and `parseInt` becomes `Number.parseInt` (S7773).
- `86400_000` was grouped wrongly; it is `86_400_000` (S7749).
These sit in session signing, CSV escaping, and GitHub App JWT
key conversion, so they were checked for equivalence rather than
assumed: old and new implementations produce identical output across
3,010 inputs including the atob round-trip, byte handling matches for
all 256 values, and createAppJwt still signs a valid JWT from a PKCS#1
key.
Also adds SECURITY.md, which sets expectations for a hosted App —
one supported version, private reporting, and an explicit note that a
repository lacking branch protection is the product working rather
than a vulnerability.
Layout: unified · split
src/auth.ts
+4 −2
| @@ -94,11 +94,13 @@ export async function fetchUserInstallationIds(userAccessToken: string): Promise |
| 94 | } |
94 | } |
| 95 | |
95 | |
| 96 | function base64UrlEncode(input: string): string { |
96 | function base64UrlEncode(input: string): string { |
| 97 | return btoa(input).replace(/\+/g, "-").replace(/\//g, "_").replace(/=+$/, ""); |
97 | // Padding is bounded to two characters, so the quantifier is too — an |
| |
98 | // unbounded `=+$` backtracks super-linearly. |
| |
99 | return btoa(input).replaceAll("+", "-").replaceAll("/", "_").replace(/={1,2}$/, ""); |
| 98 | } |
100 | } |
| 99 | |
101 | |
| 100 | function base64UrlDecode(input: string): string { |
102 | function base64UrlDecode(input: string): string { |
| 101 | const padded = input.replace(/-/g, "+").replace(/_/g, "/"); |
103 | const padded = input.replaceAll("-", "+").replaceAll("_", "/"); |
| 102 | const padding = padded.length % 4 === 0 ? "" : "=".repeat(4 - (padded.length % 4)); |
104 | const padding = padded.length % 4 === 0 ? "" : "=".repeat(4 - (padded.length % 4)); |
| 103 | return atob(padded + padding); |
105 | return atob(padded + padding); |
| 104 | } |
106 | } |
src/crypto-utils.ts
+1 −1
| @@ -1,7 +1,7 @@ |
| 1 | export function hexToBytes(hex: string): Uint8Array { |
1 | export function hexToBytes(hex: string): Uint8Array { |
| 2 | const bytes = new Uint8Array(hex.length / 2); |
2 | const bytes = new Uint8Array(hex.length / 2); |
| 3 | for (let i = 0; i < bytes.length; i++) { |
3 | for (let i = 0; i < bytes.length; i++) { |
| 4 | bytes[i] = parseInt(hex.substring(i * 2, i * 2 + 2), 16); |
4 | bytes[i] = Number.parseInt(hex.substring(i * 2, i * 2 + 2), 16); |
| 5 | } |
5 | } |
| 6 | return bytes; |
6 | return bytes; |
| 7 | } |
7 | } |
src/exporter.ts
+1 −1
| @@ -89,7 +89,7 @@ export function renderCsv(rows: EvidenceRow[]): string { |
| 89 | function csvEscape(value: string | number): string { |
89 | function csvEscape(value: string | number): string { |
| 90 | const str = String(value); |
90 | const str = String(value); |
| 91 | if (/[",\r\n]/.test(str)) { |
91 | if (/[",\r\n]/.test(str)) { |
| 92 | return `"${str.replace(/"/g, '""')}"`; |
92 | return `"${str.replaceAll('"', '""')}"`; |
| 93 | } |
93 | } |
| 94 | return str; |
94 | return str; |
| 95 | } |
95 | } |
src/github-app.ts
+2 −2
| @@ -9,7 +9,7 @@ const GITHUB_API = "https://api.github.com"; |
| 9 | // key they downloaded. |
9 | // key they downloaded. |
| 10 | function pkcs1PemToPkcs8Pem(pem: string): string { |
10 | function pkcs1PemToPkcs8Pem(pem: string): string { |
| 11 | const base64 = pem.replace(/-----(BEGIN|END) RSA PRIVATE KEY-----/g, "").replace(/\s/g, ""); |
11 | const base64 = pem.replace(/-----(BEGIN|END) RSA PRIVATE KEY-----/g, "").replace(/\s/g, ""); |
| 12 | const pkcs1 = Uint8Array.from(atob(base64), (c) => c.charCodeAt(0)); |
12 | const pkcs1 = Uint8Array.from(atob(base64), (c) => c.codePointAt(0) ?? 0); |
| 13 | |
13 | |
| 14 | const derLength = (length: number): number[] => { |
14 | const derLength = (length: number): number[] => { |
| 15 | if (length < 0x80) return [length]; |
15 | if (length < 0x80) return [length]; |
| @@ -30,7 +30,7 @@ function pkcs1PemToPkcs8Pem(pem: string): string { |
| 30 | ]); |
30 | ]); |
| 31 | |
31 | |
| 32 | let binary = ""; |
32 | let binary = ""; |
| 33 | for (const byte of pkcs8) binary += String.fromCharCode(byte); |
33 | for (const byte of pkcs8) binary += String.fromCodePoint(byte); |
| 34 | return `-----BEGIN PRIVATE KEY-----\n${btoa(binary)}\n-----END PRIVATE KEY-----`; |
34 | return `-----BEGIN PRIVATE KEY-----\n${btoa(binary)}\n-----END PRIVATE KEY-----`; |
| 35 | } |
35 | } |
| 36 | |
36 | |
src/index.ts
+3 −3
| @@ -385,8 +385,8 @@ interface CleanupResult { |
| 385 | // a no-op most of the time. |
385 | // a no-op most of the time. |
| 386 | async function runRetentionCleanup(env: Env): Promise<CleanupResult> { |
386 | async function runRetentionCleanup(env: Env): Promise<CleanupResult> { |
| 387 | const now = Date.now(); |
387 | const now = Date.now(); |
| 388 | const snapshotCutoff = new Date(now - SNAPSHOT_RETENTION_DAYS * 86400_000).toISOString(); |
388 | const snapshotCutoff = new Date(now - SNAPSHOT_RETENTION_DAYS * 86_400_000).toISOString(); |
| 389 | const exportCutoff = new Date(now - EXPORT_RETENTION_DAYS * 86400_000).toISOString(); |
389 | const exportCutoff = new Date(now - EXPORT_RETENTION_DAYS * 86_400_000).toISOString(); |
| 390 | |
390 | |
| 391 | // Delete expired export R2 objects first (their keys live in the rows). |
391 | // Delete expired export R2 objects first (their keys live in the rows). |
| 392 | const { results: expiredExports } = await env.DB.prepare( |
392 | const { results: expiredExports } = await env.DB.prepare( |
| @@ -567,7 +567,7 @@ async function handleAccessReview(request: Request, env: Env): Promise<Response> |
| 567 | const since = |
567 | const since = |
| 568 | requested && !Number.isNaN(requested.getTime()) |
568 | requested && !Number.isNaN(requested.getTime()) |
| 569 | ? requested.toISOString() |
569 | ? requested.toISOString() |
| 570 | : new Date(Date.now() - 30 * 86400_000).toISOString(); |
570 | : new Date(Date.now() - 30 * 86_400_000).toISOString(); |
| 571 | |
571 | |
| 572 | const [diff, orgRow, installations] = await Promise.all([ |
572 | const [diff, orgRow, installations] = await Promise.all([ |
| 573 | buildAccessDiff(env.DB, session.installationId, since), |
573 | buildAccessDiff(env.DB, session.installationId, since), |