fix(cli): reject missing separator after config path brackets (#109580)

* fix(cli): reject missing separator after config path brackets

* fix(cli): simplify bracket path separator validation

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
wuqxuan
2026-07-17 11:42:58 +01:00
committed by GitHub
co-authored by Peter Steinberger
parent 3a268c08df
commit d7056abbc8
2 changed files with 43 additions and 16 deletions
+36 -11
View File
@@ -420,6 +420,7 @@ function requireResolveSecretRefCall(index: number): [unknown, unknown] {
}
let registerConfigCli: typeof import("./config-cli.js").registerConfigCli;
let parseConfigSetPath: typeof import("./config-cli.js").parseConfigSetPath;
let sharedProgram: Command;
async function runConfigCommand(args: string[]) {
@@ -428,7 +429,7 @@ async function runConfigCommand(args: string[]) {
describe("config cli", () => {
beforeAll(async () => {
({ registerConfigCli } = await import("./config-cli.js"));
({ parseConfigSetPath, registerConfigCli } = await import("./config-cli.js"));
const { resolveConfigSecretTargetByPath } = await import("../secrets/target-registry.js");
resolveConfigSecretTargetByPath(["channels", "googlechat", "serviceAccount"]);
sharedProgram = new Command();
@@ -3209,22 +3210,46 @@ describe("config cli", () => {
expect(mockWriteConfigFile).not.toHaveBeenCalled();
});
it("rejects a whitespace-only segment after a bracket before a dot", async () => {
await expect(runConfigCommand(["config", "get", "agents.list[0] .id"])).rejects.toThrow(
"Invalid path (empty segment): agents.list[0] .id",
it.each([
"agents.list[0]id",
"agents.list[0] id",
"agents.list[0]\\id",
"agents.list[0] .id",
"agents.list[0] [1]",
])("rejects malformed post-bracket path %s", (configPath) => {
expect(() => parseConfigSetPath(configPath)).toThrow(
`Invalid path (missing separator after bracket): ${configPath}`,
);
});
it.each([
["get", ["config", "get", "agents.list[0]id"]],
["set", ["config", "set", "agents.list[0]id", '"renamed"']],
["unset", ["config", "unset", "agents.list[0]id"]],
[
"batch set",
[
"config",
"set",
"--batch-json",
JSON.stringify([{ path: "agents.list[0]id", value: "renamed" }]),
],
],
])("rejects malformed bracket paths for config %s", async (_command, args) => {
await expect(runConfigCommand(args)).rejects.toThrow(
"Invalid path (missing separator after bracket): agents.list[0]id",
);
expect(mockReadConfigFileSnapshot).not.toHaveBeenCalled();
expect(mockWriteConfigFile).not.toHaveBeenCalled();
});
it("rejects a whitespace-only segment after a bracket before another bracket", async () => {
await expect(runConfigCommand(["config", "get", "agents.list[0] [1]"])).rejects.toThrow(
"Invalid path (empty segment): agents.list[0] [1]",
);
expect(mockReadConfigFileSnapshot).not.toHaveBeenCalled();
expect(mockWriteConfigFile).not.toHaveBeenCalled();
it.each([
["agents.list[0].id", ["agents", "list", "0", "id"]],
["agents.list[0][1]", ["agents", "list", "0", "1"]],
["[0]", ["0"]],
])("preserves valid bracket path %s", (configPath, expected) => {
expect(parseConfigSetPath(configPath)).toEqual(expected);
});
it("preserves valid bracket path forms", async () => {
+7 -5
View File
@@ -365,9 +365,8 @@ function parseBracketPathSegment(raw: string, fullPath: string): string {
return trimmed;
}
// A buffered key with characters that are all whitespace is stray text between a
// "."/"]" and the next "."/"[" boundary (e.g. "agents.list[0] .id" or "agents.list[0] [1]").
// Pushing it would silently collapse into a different key, so reject it like an empty segment.
// A buffered key with characters that are all whitespace is stray text between
// path boundaries (for example, "gateway. .port"). Reject it like an empty segment.
function assertNotWhitespaceSegment(current: string, raw: string): void {
if (current.length > 0 && !current.trim()) {
throw new Error(`Invalid path (empty segment): ${raw}`);
@@ -382,8 +381,7 @@ function parsePath(raw: string): PathSegment[] {
const parts: string[] = [];
let current = "";
// Tracks whether a bracket segment was emitted since the last "." boundary, so
// "foo[0].bar" is accepted while empty key segments (leading/trailing/double dots,
// whitespace-only segments) are rejected instead of silently collapsed.
// "foo[0].bar" is accepted while empty key segments are rejected.
let segmentEmitted = false;
let i = 0;
while (i < trimmed.length) {
@@ -430,6 +428,10 @@ function parsePath(raw: string): PathSegment[] {
throw new Error(`Invalid path (empty "[]"): ${raw}`);
}
parts.push(parseBracketPathSegment(inside, raw));
const next = trimmed[close + 1];
if (next !== undefined && next !== "." && next !== "[") {
throw new Error(`Invalid path (missing separator after bracket): ${raw}`);
}
segmentEmitted = true;
i = close + 1;
continue;