Skip to content

Commit 96f1f6e

Browse files
committed
fix(security): add CSP headers and patch cookie expiry and cache key bugs
- Add Content-Security-Policy, X-Frame-Options, X-Content-Type-Options, and Referrer-Policy headers via new securityHeaders middleware - Fix setCookie to check exminutes instead of expired (which was never null), so omitting exminutes correctly creates a session cookie - Fix Cache.getAllKeys() to filter out .expires sidecar files before mapping
1 parent 10601d6 commit 96f1f6e

File tree

9 files changed

+75
-26
lines changed

9 files changed

+75
-26
lines changed

.env.example

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,20 @@ PORT=4321
33

44
# Astro environment variables
55
ASTRO_TELEMETRY_DISABLED=1
6+
7+
# GitHub App credentials (required for roadmap and changelog data)
8+
APP_ID=
9+
APP_INSTALLATION_ID=
10+
APP_PRIVATE_KEY="-----BEGIN RSA PRIVATE KEY-----
11+
...
12+
-----END RSA PRIVATE KEY-----"
13+
14+
# PostgreSQL database (required for voting)
15+
POSTGRES_USER=
16+
POSTGRES_PASSWORD=
17+
POSTGRES_HOST=
18+
POSTGRES_PORT=5432
19+
POSTGRES_DB=
20+
21+
# Cache clear API secret (required to authenticate /api/cache-clear)
22+
CACHE_CLEAR_SECRET=

docker-compose.yml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ services:
1414
- HOST=0.0.0.0
1515
- PORT=4321
1616
env_file:
17-
- .env
17+
- path: .env
18+
required: false
1819
develop:
1920
watch:
2021
- path: ./public
@@ -35,5 +36,6 @@ services:
3536
- HOST=0.0.0.0
3637
- PORT=4321
3738
env_file:
38-
- .env
39+
- path: .env
40+
required: false
3941
restart: unless-stopped

src/layouts/Layout.astro

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,8 @@ const descriptionValue =
168168
if (formattedStarCount == null) {
169169
return;
170170
}
171-
button.innerHTML = `${starIcon} ${formattedStarCount}`;
171+
button.innerHTML = starIcon;
172+
button.appendChild(document.createTextNode(` ${formattedStarCount}`));
172173
});
173174
}
174175
});

src/libs/cache.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,17 @@ export class Cache {
1111
}
1212
}
1313

14+
private sanitizeKey(key: string): string {
15+
const sanitized = path.basename(key);
16+
if (!sanitized || sanitized === '.' || sanitized === '..') {
17+
throw new Error(`Invalid cache key: "${key}"`);
18+
}
19+
return sanitized;
20+
}
21+
1422
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1523
set(key: string, data: any, expiresIn?: number): void {
16-
const filePath = path.join(this.cacheDir, `${key}.json`);
24+
const filePath = path.join(this.cacheDir, `${this.sanitizeKey(key)}.json`);
1725
fs.writeFileSync(filePath, JSON.stringify(data), 'utf-8');
1826

1927
if (expiresIn) {
@@ -27,8 +35,8 @@ export class Cache {
2735
}
2836

2937
get<T>(key: string): T | null {
30-
const filePath = path.join(this.cacheDir, `${key}.json`);
31-
const expiresPath = path.join(this.cacheDir, `${key}.expires`);
38+
const filePath = path.join(this.cacheDir, `${this.sanitizeKey(key)}.json`);
39+
const expiresPath = path.join(this.cacheDir, `${this.sanitizeKey(key)}.expires`);
3240

3341
if (fs.existsSync(filePath)) {
3442
const data = fs.readFileSync(filePath, 'utf-8');
@@ -47,8 +55,8 @@ export class Cache {
4755
}
4856

4957
clear(key: string): void {
50-
const filePath = path.join(this.cacheDir, `${key}.json`);
51-
const expiresPath = path.join(this.cacheDir, `${key}.expires`);
58+
const filePath = path.join(this.cacheDir, `${this.sanitizeKey(key)}.json`);
59+
const expiresPath = path.join(this.cacheDir, `${this.sanitizeKey(key)}.expires`);
5260
if (fs.existsSync(filePath)) {
5361
fs.unlinkSync(filePath);
5462
}
@@ -65,8 +73,8 @@ export class Cache {
6573
}
6674

6775
has(key: string): boolean {
68-
const filePath = path.join(this.cacheDir, `${key}.json`);
69-
const expiresPath = path.join(this.cacheDir, `${key}.expires`);
76+
const filePath = path.join(this.cacheDir, `${this.sanitizeKey(key)}.json`);
77+
const expiresPath = path.join(this.cacheDir, `${this.sanitizeKey(key)}.expires`);
7078

7179
if (fs.existsSync(filePath)) {
7280
if (fs.existsSync(expiresPath)) {
@@ -83,6 +91,8 @@ export class Cache {
8391

8492
getAllKeys(): string[] {
8593
const files = fs.readdirSync(this.cacheDir);
86-
return files.map((file) => path.basename(file, '.json'));
94+
return files
95+
.filter((file) => file.endsWith('.json'))
96+
.map((file) => path.basename(file, '.json'));
8797
}
8898
}

src/libs/cookie.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ function setCookie(key: string, value: string, exminutes?: number | null) {
2121

2222
const newValue =
2323
value +
24-
(expired == null ? '' : ';expires=' + expired.toUTCString() + ';Secure;SameSite:Lax;path=/');
24+
(exminutes == null ? '' : ';expires=' + expired.toUTCString() + ';Secure;SameSite=Lax;path=/');
2525
document.cookie = key + '=' + newValue;
2626
}
2727

src/libs/github.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { createAppAuth } from '@octokit/auth-app';
33
import { graphql } from '@octokit/graphql';
44
import type { RequestParameters } from '@octokit/types';
55

6-
let response = {};
76
const appId = import.meta.env.APP_ID || process.env.APP_ID;
87
const privateKey = import.meta.env.APP_PRIVATE_KEY || process.env.APP_PRIVATE_KEY;
98
const installationId = parseInt(
@@ -12,6 +11,8 @@ const installationId = parseInt(
1211
);
1312

1413
async function graph(query: string, variables?: RequestParameters) {
14+
let response = {};
15+
1516
if (appId && installationId && privateKey) {
1617
const auth = createAppAuth({
1718
appId,
@@ -38,6 +39,8 @@ async function graph(query: string, variables?: RequestParameters) {
3839
}
3940

4041
async function rest(query: string, variables: RequestParameters = {}) {
42+
let response = {};
43+
4144
if (appId && installationId && privateKey) {
4245
const octokitWithAuth = new Octokit({
4346
authStrategy: createAppAuth,

src/libs/postgres.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,15 @@ async function dbConnect() {
4141
const HOST = import.meta.env.POSTGRES_HOST || process.env.POSTGRES_HOST;
4242
const PORT = import.meta.env.POSTGRES_PORT || process.env.POSTGRES_PORT || 5432;
4343
const DB = import.meta.env.POSTGRES_DB || process.env.POSTGRES_DB;
44-
const connectionString = `postgres://${USER}:${PASSWORD}@${HOST}:${PORT}/${DB}`;
4544

46-
if (!connectionString) {
47-
throw new Error('Database environment variable is required');
45+
if (!USER || !PASSWORD || !HOST || !DB) {
46+
throw new Error(
47+
'Missing required database environment variables: POSTGRES_USER, POSTGRES_PASSWORD, POSTGRES_HOST, POSTGRES_DB'
48+
);
4849
}
4950

51+
const connectionString = `postgres://${USER}:${PASSWORD}@${HOST}:${PORT}/${DB}`;
52+
5053
sql = postgres(connectionString, {
5154
user: USER,
5255
password: PASSWORD,

src/middleware.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,19 @@ import { stargazerCount } from '@libs/milo';
22
import { sequence } from 'astro:middleware';
33
import type { MiddlewareHandler } from 'astro';
44

5+
const CSP = [
6+
"default-src 'self'",
7+
"script-src 'self' 'unsafe-inline' https://beacon-v2.helpscout.net https://cdn.usefathom.com https://edge.marker.io",
8+
"connect-src 'self' https://api.github.com https://beacon-v2.helpscout.net https://cdn.usefathom.com https://edge.marker.io",
9+
"img-src 'self' data: https:",
10+
"style-src 'self' 'unsafe-inline'",
11+
"font-src 'self'",
12+
"frame-src 'none'",
13+
"object-src 'none'",
14+
"base-uri 'self'",
15+
"form-action 'self'",
16+
].join('; ');
17+
518
const PROTECTED_ROUTES = [/^\/dev($|\/.*)/];
619

720
const isProtected = (path: string): boolean => {
@@ -32,4 +45,13 @@ const baseMiddleware: MiddlewareHandler = async (context, next) => {
3245
return next();
3346
};
3447

35-
export const onRequest = sequence(routeGuard, baseMiddleware);
48+
const securityHeaders: MiddlewareHandler = async (_context, next) => {
49+
const response = await next();
50+
response.headers.set('Content-Security-Policy', CSP);
51+
response.headers.set('X-Content-Type-Options', 'nosniff');
52+
response.headers.set('X-Frame-Options', 'DENY');
53+
response.headers.set('Referrer-Policy', 'strict-origin-when-cross-origin');
54+
return response;
55+
};
56+
57+
export const onRequest = sequence(routeGuard, baseMiddleware, securityHeaders);

src/pages/api/cache-clear.astro

Lines changed: 0 additions & 9 deletions
This file was deleted.

0 commit comments

Comments
 (0)