Skip to content

Commit 772daba

Browse files
lucianfialhoclaude
andcommitted
fix: harden multi-header auth — reject CRLF, warn on empty $VAR
- parseHeaderFlag now rejects CR/LF in name/value (RFC 7230 header injection) and enforces RFC-valid token chars in header names. - resolveEnvVar warns to stderr when $VAR resolves to unset or empty — silent empty headers produce confusing 401s ("auth wrong" when it's "env unset"). - index.ts parseHeaderArgs now delegates to parseHeaderFlag so CLI-time argv parsing shares the same validation as `auth login`. - .gitignore: ignore .claude/ harness state. Addresses code-review feedback on PR #21. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 18b4ac7 commit 772daba

5 files changed

Lines changed: 66 additions & 20 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,4 @@ dist/
33
*.tgz
44
.env
55
.DS_Store
6+
.claude/

src/auth/auth.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,18 @@ describe("parseHeaderFlag", () => {
192192
it("returns null for empty name", () => {
193193
expect(parseHeaderFlag(": value")).toBeNull();
194194
});
195+
196+
it("rejects CR/LF in value (header injection)", () => {
197+
expect(parseHeaderFlag("X-Key: bad\r\nEvil: yes")).toBeNull();
198+
expect(parseHeaderFlag("X-Key: bad\nEvil: yes")).toBeNull();
199+
expect(parseHeaderFlag("X-Key: bad\rEvil: yes")).toBeNull();
200+
});
201+
202+
it("rejects invalid characters in header name", () => {
203+
expect(parseHeaderFlag("X Key: v")).toBeNull();
204+
expect(parseHeaderFlag("X\tKey: v")).toBeNull();
205+
expect(parseHeaderFlag("X:Key: v")).toEqual({ name: "X", value: "Key: v" });
206+
});
195207
});
196208

197209
describe("detectApiKeyHeaders", () => {
@@ -271,6 +283,32 @@ describe("resolveAuth with multi-header", () => {
271283
expect(auth.type).toBe("bearer");
272284
expect(auth.value).toBe("sk-1");
273285
});
286+
287+
it("warns to stderr when $VAR in header resolves to empty", async () => {
288+
const stderr = vi.spyOn(console, "error").mockImplementation(() => {});
289+
const auth = await resolveAuth(
290+
{ headers: { "X-VTEX-API-AppKey": "$MISSING_VAR" } },
291+
specDualHeader,
292+
{}
293+
);
294+
expect(auth.headers?.["X-VTEX-API-AppKey"]).toBe("");
295+
const logged = stderr.mock.calls.map((c) => String(c[0])).join("\n");
296+
expect(logged).toMatch(/Warning.*MISSING_VAR.*unset/);
297+
expect(logged).toContain("X-VTEX-API-AppKey");
298+
stderr.mockRestore();
299+
});
300+
301+
it("warns when $VAR resolves to empty string", async () => {
302+
const stderr = vi.spyOn(console, "error").mockImplementation(() => {});
303+
await resolveAuth(
304+
{ headers: { "X-Key": "$EMPTY_VAR" } },
305+
specDualHeader,
306+
{ EMPTY_VAR: "" }
307+
);
308+
const logged = stderr.mock.calls.map((c) => String(c[0])).join("\n");
309+
expect(logged).toMatch(/Warning.*EMPTY_VAR.*empty/);
310+
stderr.mockRestore();
311+
});
274312
});
275313

276314
describe("maskToken", () => {

src/auth/flags.ts

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,25 @@ export async function resolveAuth(
2222
if (flags.headers && Object.keys(flags.headers).length > 0) {
2323
const resolved: Record<string, string> = {};
2424
for (const [k, v] of Object.entries(flags.headers)) {
25-
resolved[k] = resolveEnvVar(v, env);
25+
resolved[k] = resolveEnvVar(v, env, `--header "${k}"`);
2626
}
2727
return { type: "headers", value: "", headers: resolved };
2828
}
2929
if (flags.token) {
30-
return { type: "bearer", value: resolveEnvVar(flags.token, env) };
30+
return { type: "bearer", value: resolveEnvVar(flags.token, env, "--token") };
3131
}
3232
if (flags.apiKey) {
3333
const headerName = detectApiKeyHeader(spec) ?? "X-API-Key";
34-
return { type: "apiKey", value: resolveEnvVar(flags.apiKey, env), headerName };
34+
return { type: "apiKey", value: resolveEnvVar(flags.apiKey, env, "--api-key"), headerName };
3535
}
3636
if (flags.authHeader) {
37-
return { type: "bearer", value: resolveEnvVar(flags.authHeader, env) };
37+
return { type: "bearer", value: resolveEnvVar(flags.authHeader, env, "--auth-header") };
3838
}
3939

4040
// Priority 2: .toclirc auth config
4141
if (flags.rcAuthToken) {
4242
const type = (flags.rcAuthType as AuthConfig["type"]) ?? "bearer";
43-
return { type, value: resolveEnvVar(flags.rcAuthToken, env) };
43+
return { type, value: resolveEnvVar(flags.rcAuthToken, env, ".toclirc auth.token") };
4444
}
4545
if (flags.rcAuthEnvVar) {
4646
const envVal = env[flags.rcAuthEnvVar];
@@ -69,13 +69,13 @@ export async function resolveAuth(
6969
if (profile.type === "headers" && profile.headers) {
7070
const resolved: Record<string, string> = {};
7171
for (const [k, v] of Object.entries(profile.headers)) {
72-
resolved[k] = resolveEnvVar(v, env);
72+
resolved[k] = resolveEnvVar(v, env, `profile '${profileName}' header "${k}"`);
7373
}
7474
return { type: "headers", value: "", headers: resolved };
7575
}
7676
return {
7777
type: profile.type,
78-
value: resolveEnvVar(profile.value, env),
78+
value: resolveEnvVar(profile.value, env, `profile '${profileName}'`),
7979
headerName: profile.headerName,
8080
};
8181
}
@@ -115,11 +115,19 @@ function detectApiKeyHeader(spec: OpenAPISpec): string | undefined {
115115
return undefined;
116116
}
117117

118-
function resolveEnvVar(value: string, env: NodeJS.ProcessEnv): string {
119-
// Replace $VAR or ${VAR} with env values
118+
function resolveEnvVar(value: string, env: NodeJS.ProcessEnv, context?: string): string {
119+
// Replace $VAR or ${VAR} with env values. Warn when a reference resolves to empty —
120+
// silent empty headers confuse downstream 401s ("auth wrong" when it's "env unset").
120121
return value.replace(/\$\{([^}]+)\}|\$([A-Z_][A-Z0-9_]*)/g, (_, braced, plain) => {
121122
const name = braced ?? plain;
122-
return env[name] ?? "";
123+
const resolved = env[name];
124+
if (resolved === undefined || resolved === "") {
125+
console.error(
126+
`Warning: env var $${name} is ${resolved === undefined ? "unset" : "empty"}${context ? ` (used in ${context})` : ""}.`
127+
);
128+
return "";
129+
}
130+
return resolved;
123131
});
124132
}
125133

src/auth/headers.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,9 @@ export function parseHeaderFlag(raw: string): ParsedHeader | null {
1414
const name = raw.slice(0, idx).trim();
1515
const value = raw.slice(idx + 1).trim();
1616
if (!name) return null;
17+
// Reject CR/LF — header injection (RFC 7230 §3.2.4 forbids them in values/names)
18+
if (/[\r\n]/.test(name) || /[\r\n]/.test(value)) return null;
19+
// Header names per RFC 7230: token chars only (no whitespace, no control chars)
20+
if (!/^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$/.test(name)) return null;
1721
return { name, value };
1822
}

src/index.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { executeRequest } from "./executor/http.js";
77
import { formatOutput } from "./output/formatters.js";
88
import { registerAuthCommands } from "./auth/commands.js";
99
import { resolveAuth as resolveAuthFromFlags } from "./auth/flags.js";
10+
import { parseHeaderFlag } from "./auth/headers.js";
1011
import { registerInitCommand } from "./config/init.js";
1112
import { registerUseCommand } from "./templates/commands.js";
1213
import { loadConfig, resolveConfig } from "./config/rc.js";
@@ -409,18 +410,12 @@ function parseHeaderArgs(args: string[]): Record<string, string> | undefined {
409410
if (raws.length === 0) return undefined;
410411
const headers: Record<string, string> = {};
411412
for (const raw of raws) {
412-
const idx = raw.indexOf(":");
413-
if (idx === -1) {
414-
console.error(`Error: invalid --header '${raw}'. Expected "Name: Value".`);
413+
const parsed = parseHeaderFlag(raw);
414+
if (!parsed) {
415+
console.error(`Error: invalid --header '${raw}'. Expected "Name: Value" with RFC-valid name and no CR/LF.`);
415416
process.exit(1);
416417
}
417-
const name = raw.slice(0, idx).trim();
418-
const value = raw.slice(idx + 1).trim();
419-
if (!name) {
420-
console.error(`Error: invalid --header '${raw}'. Missing name.`);
421-
process.exit(1);
422-
}
423-
headers[name] = value;
418+
headers[parsed.name] = parsed.value;
424419
}
425420
return headers;
426421
}

0 commit comments

Comments
 (0)