Skip to content

Commit 27451ef

Browse files
authored
Resolve release CodeQL findings (#39)
## What changed - pin AWS integration credentials to the exact API Gateway ID and Region returned for the ephemeral CloudFormation stack - remove response bodies, API key identifiers, and arbitrary error messages from cleanup logs - read bounded template files through one `O_NOFOLLOW` file descriptor for both metadata checks and content reads - add endpoint-confusion regression tests and release notes ## Why CodeQL's first scan of the complete v0.1.0 pull request identified three new flows: a dynamic integration-test destination, clear-text cleanup logging, and a `stat(path)` / `readFile(path)` race. Even though the integration destination originates from a dedicated test stack, it carries a bootstrap credential and should fail closed if the URL differs from that exact stack. The logging and file race findings are actionable directly. ## Impact The AWS integration workflow now independently reads the `HttpApi` physical ID and the script accepts only its exact commercial API Gateway HTTPS hostname. Cleanup diagnostics remain useful without retaining response bodies or key identifiers. Template manifest behavior is unchanged, but the size/type check and read now operate on the same opened file. ## Validation - `npm run check` - `npm test` — 22 files, 146 tests - `npm run build` - endpoint-confusion and bootstrap-key output probes - `npm audit --omit=dev --audit-level=high` — 0 vulnerabilities - `cfn-lint==1.53.2 template.yaml` - Redocly 2.12.5 OpenAPI lint - `actionlint` - `gitleaks git --staged` - signed commit with DCO sign-off Closes CodeQL findings #3, #4, and #5 on release PR #30. Signed-off-by: Yusuke Hayashi <yusuke8h@gmail.com>
1 parent 629041e commit 27451ef

7 files changed

Lines changed: 134 additions & 21 deletions

File tree

.github/workflows/aws-integration.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,21 @@ jobs:
121121
--output text
122122
}
123123
api_base_url="$(stack_output ApiBaseUrl)"
124+
http_api_id="$(
125+
aws cloudformation describe-stack-resource \
126+
--stack-name "$STACK_NAME" \
127+
--logical-resource-id HttpApi \
128+
--region "$AWS_REGION" \
129+
--query StackResourceDetail.PhysicalResourceId \
130+
--output text
131+
)"
124132
bootstrap_secret_arn="$(stack_output BootstrapSecretArn)"
125133
payload_bucket="$(stack_output PayloadBucketName)"
126134
data_table="$(stack_output TableName)"
127135
schedule_group="$(stack_output EmailScheduleGroupName)"
128136
for value in \
129137
"$api_base_url" \
138+
"$http_api_id" \
130139
"$bootstrap_secret_arn" \
131140
"$payload_bucket" \
132141
"$data_table" \
@@ -138,6 +147,7 @@ jobs:
138147
done
139148
{
140149
echo "API_BASE_URL=$api_base_url"
150+
echo "HTTP_API_ID=$http_api_id"
141151
echo "BOOTSTRAP_SECRET_ARN=$bootstrap_secret_arn"
142152
echo "PAYLOAD_BUCKET=$payload_bucket"
143153
echo "DATA_TABLE=$data_table"
@@ -156,6 +166,7 @@ jobs:
156166
)"
157167
echo "::add-mask::$bootstrap_key"
158168
HAYASEND_BASE_URL="$API_BASE_URL" \
169+
HAYASEND_EXPECTED_API_ID="$HTTP_API_ID" \
159170
HAYASEND_BOOTSTRAP_KEY="$bootstrap_key" \
160171
node scripts/aws-integration.mjs
161172

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ No unreleased changes.
2020
lookup so unauthenticated input cannot turn storage limits into API errors.
2121
- Preflight every email in a strict batch before persisting or queueing any
2222
message, preventing partial sends when a template or attachment is invalid.
23+
- Pin integration credentials to the expected API Gateway endpoint, redact
24+
cleanup failures, and read template files through race-safe descriptors.
2325
- Initial Resend-compatible sending, scheduling, receiving, webhook, and AWS
2426
deployment foundation.
2527
- Add Resend-compatible hosted templates with aliases, typed variables,
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export function normalizeApiGatewayBaseUrl(
2+
value: string,
3+
expectedApiId: string,
4+
region: string,
5+
): string;

scripts/aws-integration-safety.mjs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
export function normalizeApiGatewayBaseUrl(value, expectedApiId, region) {
2+
if (
3+
!/^[a-z0-9]+$/.test(expectedApiId) ||
4+
!/^[a-z]{2}(?:-[a-z0-9]+)+-[0-9]+$/.test(region)
5+
) {
6+
throw new Error("The expected API Gateway identifier or Region is invalid.");
7+
}
8+
let endpoint;
9+
try {
10+
endpoint = new URL(value);
11+
} catch {
12+
throw new Error("HAYASEND_BASE_URL must be an absolute URL.");
13+
}
14+
const expectedHostname =
15+
`${expectedApiId}.execute-api.${region}.amazonaws.com`;
16+
if (
17+
endpoint.protocol !== "https:" ||
18+
endpoint.hostname !== expectedHostname ||
19+
endpoint.port ||
20+
endpoint.username ||
21+
endpoint.password ||
22+
endpoint.pathname !== "/" ||
23+
endpoint.search ||
24+
endpoint.hash
25+
) {
26+
throw new Error(
27+
"HAYASEND_BASE_URL must be the expected dedicated API Gateway endpoint.",
28+
);
29+
}
30+
return `https://${expectedHostname}`;
31+
}

scripts/aws-integration.mjs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import assert from "node:assert/strict";
22
import { createHash } from "node:crypto";
3+
import { normalizeApiGatewayBaseUrl } from "./aws-integration-safety.mjs";
34

45
function requiredEnvironment(name) {
56
const value = process.env[name];
@@ -9,7 +10,11 @@ function requiredEnvironment(name) {
910
return value;
1011
}
1112

12-
const baseUrl = requiredEnvironment("HAYASEND_BASE_URL").replace(/\/$/, "");
13+
const baseUrl = normalizeApiGatewayBaseUrl(
14+
requiredEnvironment("HAYASEND_BASE_URL"),
15+
requiredEnvironment("HAYASEND_EXPECTED_API_ID"),
16+
requiredEnvironment("AWS_REGION"),
17+
);
1318
const bootstrapKey = requiredEnvironment("HAYASEND_BOOTSTRAP_KEY");
1419
const runId = (process.env.GITHUB_RUN_ID ?? String(Date.now())).replace(
1520
/[^a-zA-Z0-9-]/g,
@@ -35,7 +40,7 @@ async function api(
3540
const raw = await response.text();
3641
if (response.status !== expectedStatus) {
3742
throw new Error(
38-
`${method} ${path} returned ${response.status}, expected ${expectedStatus}: ${raw.slice(0, 1_000)}`,
43+
`${method} ${path} returned ${response.status}, expected ${expectedStatus}.`,
3944
);
4045
}
4146
return raw ? JSON.parse(raw) : undefined;
@@ -44,9 +49,8 @@ async function api(
4449
async function bestEffort(label, operation) {
4550
try {
4651
await operation();
47-
} catch (error) {
48-
const message = error instanceof Error ? error.message : String(error);
49-
console.warn(`Cleanup warning for ${label}: ${message}`);
52+
} catch {
53+
console.warn(`Cleanup warning: ${label} could not be removed.`);
5054
}
5155
}
5256

@@ -280,7 +284,7 @@ try {
280284
);
281285
}
282286
for (const apiKeyId of created.apiKeyIds.reverse()) {
283-
await bestEffort(`API key ${apiKeyId}`, () =>
287+
await bestEffort("an API key", () =>
284288
api("DELETE", `/api-keys/${apiKeyId}`, bootstrapKey),
285289
);
286290
}

src/cli-templates.ts

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { readFile, realpath, stat } from "node:fs/promises";
1+
import { constants } from "node:fs";
2+
import { open, realpath } from "node:fs/promises";
23
import { dirname, isAbsolute, relative, resolve } from "node:path";
34
import { z } from "zod";
45
import { createTemplateSchema } from "./schemas.js";
@@ -145,18 +146,23 @@ function formatZodError(error: z.ZodError) {
145146
}
146147

147148
async function readBoundedFile(path: string, maximumBytes: number) {
148-
const metadata = await stat(path);
149-
if (!metadata.isFile()) {
150-
throw new Error(`Expected a regular file: ${path}`);
151-
}
152-
if (metadata.size > maximumBytes) {
153-
throw new Error(`File exceeds the ${maximumBytes}-byte limit: ${path}`);
154-
}
155-
const content = await readFile(path, "utf8");
156-
if (Buffer.byteLength(content, "utf8") > maximumBytes) {
157-
throw new Error(`File exceeds the ${maximumBytes}-byte limit: ${path}`);
149+
const file = await open(path, constants.O_RDONLY | constants.O_NOFOLLOW);
150+
try {
151+
const metadata = await file.stat();
152+
if (!metadata.isFile()) {
153+
throw new Error(`Expected a regular file: ${path}`);
154+
}
155+
if (metadata.size > maximumBytes) {
156+
throw new Error(`File exceeds the ${maximumBytes}-byte limit: ${path}`);
157+
}
158+
const content = await file.readFile({ encoding: "utf8" });
159+
if (Buffer.byteLength(content, "utf8") > maximumBytes) {
160+
throw new Error(`File exceeds the ${maximumBytes}-byte limit: ${path}`);
161+
}
162+
return content;
163+
} finally {
164+
await file.close();
158165
}
159-
return content;
160166
}
161167

162168
async function resolveContentFile(
@@ -260,7 +266,8 @@ export async function loadTemplateManifest(
260266
cwd: string,
261267
configuredPath = "hayasend.templates.json",
262268
) {
263-
const manifestPath = resolve(cwd, configuredPath);
269+
const configuredManifestPath = resolve(cwd, configuredPath);
270+
const manifestPath = await realpath(configuredManifestPath);
264271
const source = await readBoundedFile(manifestPath, MAX_MANIFEST_BYTES);
265272
let untrusted: unknown;
266273
try {
@@ -278,13 +285,13 @@ export async function loadTemplateManifest(
278285
`Template manifest is invalid: ${formatZodError(parsed.error)}`,
279286
);
280287
}
281-
const root = await realpath(dirname(manifestPath));
288+
const root = dirname(manifestPath);
282289
const templates = await Promise.all(
283290
parsed.data.templates.map((template) =>
284291
loadDesiredTemplate(root, template),
285292
),
286293
);
287-
return { path: manifestPath, templates };
294+
return { path: configuredManifestPath, templates };
288295
}
289296

290297
export function parseRemoteTemplate(value: unknown): RemoteTemplate {
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { describe, expect, it } from "vitest";
2+
import { normalizeApiGatewayBaseUrl } from "../scripts/aws-integration-safety.mjs";
3+
4+
describe("AWS integration network safety", () => {
5+
it("accepts only the expected regional API Gateway endpoint", () => {
6+
expect(
7+
normalizeApiGatewayBaseUrl(
8+
"https://abc123def4.execute-api.ap-northeast-1.amazonaws.com/",
9+
"abc123def4",
10+
"ap-northeast-1",
11+
),
12+
).toBe(
13+
"https://abc123def4.execute-api.ap-northeast-1.amazonaws.com",
14+
);
15+
16+
for (const endpoint of [
17+
"http://abc123def4.execute-api.ap-northeast-1.amazonaws.com",
18+
"https://other12345.execute-api.ap-northeast-1.amazonaws.com",
19+
"https://abc123def4.execute-api.us-east-1.amazonaws.com",
20+
"https://abc123def4.execute-api.ap-northeast-1.amazonaws.com:444",
21+
"https://user:secret@abc123def4.execute-api.ap-northeast-1.amazonaws.com",
22+
"https://abc123def4.execute-api.ap-northeast-1.amazonaws.com/stage",
23+
"https://abc123def4.execute-api.ap-northeast-1.amazonaws.com?target=other",
24+
"https://abc123def4.execute-api.ap-northeast-1.amazonaws.com#fragment",
25+
"https://abc123def4.execute-api.ap-northeast-1.amazonaws.com.attacker.example",
26+
]) {
27+
expect(() =>
28+
normalizeApiGatewayBaseUrl(
29+
endpoint,
30+
"abc123def4",
31+
"ap-northeast-1",
32+
),
33+
).toThrow("expected dedicated API Gateway endpoint");
34+
}
35+
});
36+
37+
it("rejects malformed expected deployment identifiers", () => {
38+
expect(() =>
39+
normalizeApiGatewayBaseUrl(
40+
"https://abc123def4.execute-api.ap-northeast-1.amazonaws.com",
41+
"abc123def4.example",
42+
"ap-northeast-1",
43+
),
44+
).toThrow("identifier or Region is invalid");
45+
expect(() =>
46+
normalizeApiGatewayBaseUrl(
47+
"https://abc123def4.execute-api.ap-northeast-1.amazonaws.com",
48+
"abc123def4",
49+
"ap-northeast-1.amazonaws.com",
50+
),
51+
).toThrow("identifier or Region is invalid");
52+
});
53+
});

0 commit comments

Comments
 (0)