fix: harden env var substitution parsing (#1044) (thanks @sebslight)
This commit is contained in:
@@ -24,6 +24,7 @@
|
|||||||
- Telegram: allow reply-chain messages to bypass mention gating in groups. (#1038) — thanks @adityashaw2.
|
- Telegram: allow reply-chain messages to bypass mention gating in groups. (#1038) — thanks @adityashaw2.
|
||||||
- Cron: isolated cron jobs now start a fresh session id on every run to prevent context buildup.
|
- Cron: isolated cron jobs now start a fresh session id on every run to prevent context buildup.
|
||||||
- Docs: add `/help` hub, Node/npm PATH guide, and expand directory CLI docs.
|
- Docs: add `/help` hub, Node/npm PATH guide, and expand directory CLI docs.
|
||||||
|
- Config: support env var substitution in config values. (#1044) — thanks @sebslight.
|
||||||
|
|
||||||
### Fixes
|
### Fixes
|
||||||
- Messages: `/stop` now hard-aborts queued followups and sub-agent runs; suppress zero-count stop notes.
|
- Messages: `/stop` now hard-aborts queued followups and sub-agent runs; suppress zero-count stop notes.
|
||||||
|
|||||||
@@ -129,10 +129,25 @@ describe("resolveConfigEnvVars", () => {
|
|||||||
expect(result).toEqual({ key: "resolved/${LITERAL}" });
|
expect(result).toEqual({ key: "resolved/${LITERAL}" });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("handles escaped and unescaped of the same var (escaped first)", () => {
|
||||||
|
const result = resolveConfigEnvVars({ key: "$${FOO} ${FOO}" }, { FOO: "bar" });
|
||||||
|
expect(result).toEqual({ key: "${FOO} bar" });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("handles escaped and unescaped of the same var (unescaped first)", () => {
|
||||||
|
const result = resolveConfigEnvVars({ key: "${FOO} $${FOO}" }, { FOO: "bar" });
|
||||||
|
expect(result).toEqual({ key: "bar ${FOO}" });
|
||||||
|
});
|
||||||
|
|
||||||
it("handles multiple escaped vars", () => {
|
it("handles multiple escaped vars", () => {
|
||||||
const result = resolveConfigEnvVars({ key: "$${A}:$${B}" }, {});
|
const result = resolveConfigEnvVars({ key: "$${A}:$${B}" }, {});
|
||||||
expect(result).toEqual({ key: "${A}:${B}" });
|
expect(result).toEqual({ key: "${A}:${B}" });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("does not unescape $${VAR} sequences from env values", () => {
|
||||||
|
const result = resolveConfigEnvVars({ key: "${FOO}" }, { FOO: "$${BAR}" });
|
||||||
|
expect(result).toEqual({ key: "$${BAR}" });
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("non-matching patterns unchanged", () => {
|
describe("non-matching patterns unchanged", () => {
|
||||||
|
|||||||
@@ -22,10 +22,7 @@
|
|||||||
|
|
||||||
// Pattern for valid uppercase env var names: starts with letter or underscore,
|
// Pattern for valid uppercase env var names: starts with letter or underscore,
|
||||||
// followed by letters, numbers, or underscores (all uppercase)
|
// followed by letters, numbers, or underscores (all uppercase)
|
||||||
const ENV_VAR_PATTERN = /\$\{([A-Z_][A-Z0-9_]*)\}/g;
|
const ENV_VAR_NAME_PATTERN = /^[A-Z_][A-Z0-9_]*$/;
|
||||||
|
|
||||||
// Pattern for escaped env vars: $${...} -> ${...}
|
|
||||||
const ESCAPED_PATTERN = /\$\$\{([A-Z_][A-Z0-9_]*)\}/g;
|
|
||||||
|
|
||||||
export class MissingEnvVarError extends Error {
|
export class MissingEnvVarError extends Error {
|
||||||
constructor(
|
constructor(
|
||||||
@@ -46,26 +43,65 @@ function isPlainObject(value: unknown): value is Record<string, unknown> {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function readEnvVarName(
|
||||||
|
value: string,
|
||||||
|
start: number,
|
||||||
|
): { name: string | null; end: number } {
|
||||||
|
const end = value.indexOf("}", start);
|
||||||
|
if (end === -1) {
|
||||||
|
return { name: null, end: -1 };
|
||||||
|
}
|
||||||
|
|
||||||
|
const name = value.slice(start, end);
|
||||||
|
if (!ENV_VAR_NAME_PATTERN.test(name)) {
|
||||||
|
return { name: null, end: -1 };
|
||||||
|
}
|
||||||
|
|
||||||
|
return { name, end };
|
||||||
|
}
|
||||||
|
|
||||||
function substituteString(value: string, env: NodeJS.ProcessEnv, configPath: string): string {
|
function substituteString(value: string, env: NodeJS.ProcessEnv, configPath: string): string {
|
||||||
// First pass: substitute real env vars
|
let result = "";
|
||||||
const substituted = value.replace(ENV_VAR_PATTERN, (match, varName: string) => {
|
|
||||||
// Check if this is actually an escaped var (preceded by $)
|
for (let i = 0; i < value.length; i += 1) {
|
||||||
// We handle this in the second pass, so check the original string
|
const char = value[i];
|
||||||
const idx = value.indexOf(match);
|
if (char !== "$") {
|
||||||
if (idx > 0 && value[idx - 1] === "$") {
|
result += char;
|
||||||
// This will be handled by ESCAPED_PATTERN, return as-is for now
|
continue;
|
||||||
return match;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const envValue = env[varName];
|
const next = value[i + 1];
|
||||||
if (envValue === undefined || envValue === "") {
|
const afterNext = value[i + 2];
|
||||||
throw new MissingEnvVarError(varName, configPath);
|
|
||||||
}
|
|
||||||
return envValue;
|
|
||||||
});
|
|
||||||
|
|
||||||
// Second pass: convert escaped $${VAR} to literal ${VAR}
|
// Escaped: $${VAR} -> ${VAR}
|
||||||
return substituted.replace(ESCAPED_PATTERN, (_, varName: string) => `\${${varName}}`);
|
if (next === "$" && afterNext === "{") {
|
||||||
|
const { name, end } = readEnvVarName(value, i + 3);
|
||||||
|
if (name !== null) {
|
||||||
|
result += `\${${name}}`;
|
||||||
|
i = end;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Substitution: ${VAR} -> value
|
||||||
|
if (next === "{") {
|
||||||
|
const { name, end } = readEnvVarName(value, i + 2);
|
||||||
|
if (name !== null) {
|
||||||
|
const envValue = env[name];
|
||||||
|
if (envValue === undefined || envValue === "") {
|
||||||
|
throw new MissingEnvVarError(name, configPath);
|
||||||
|
}
|
||||||
|
result += envValue;
|
||||||
|
i = end;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Leave untouched if not a recognized pattern
|
||||||
|
result += char;
|
||||||
|
}
|
||||||
|
|
||||||
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
function substituteAny(value: unknown, env: NodeJS.ProcessEnv, path: string): unknown {
|
function substituteAny(value: unknown, env: NodeJS.ProcessEnv, path: string): unknown {
|
||||||
|
|||||||
Reference in New Issue
Block a user