Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Fixed

- Paths and task input are now inserted verbatim where GJC substitutes a placeholder. Five call sites passed a variable as the replacement argument of `String.prototype.replace`/`replaceAll`, so JavaScript expanded `$&`, `$$`, `` $` `` and `$'` inside the value instead of copying it: a plugin installed under `a$&b` launched its MCP server from `a${CLAUDE_PLUGIN_ROOT}b/...`, a repository named `q$'z` had its launch worktrees resolved to `q/.worktreesz/.worktrees`, and a workflow command run with `sed s/a/$&/g` received `sed s/a/$@/g`. Affected: plugin MCP `command`/`args`/`env`/`cwd` root substitution, the launch-worktree bucket and coordinator managed-worktree `{repo}` templates, `~` expansion for `gjc auth-broker import`, and `$@` in workflow command instructions.
2 changes: 1 addition & 1 deletion packages/coding-agent/src/cli/auth-broker-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ async function runImport(flags: AuthBrokerCommandArgs["flags"]): Promise<void> {
if (!target) {
throw new Error("Usage: gjc auth-broker import <file|dir> [--provider=<id>] [--include-disabled] [--dry-run]");
}
const resolvedTarget = path.resolve(target.startsWith("~") ? target.replace(/^~/, os.homedir()) : target);
const resolvedTarget = path.resolve(target.startsWith("~") ? target.replace(/^~/, () => os.homedir()) : target);
const { entries, skipped } = await loadImportPlan(resolvedTarget, flags.provider, flags.includeDisabled === true);

if (flags.json) {
Expand Down
5 changes: 3 additions & 2 deletions packages/coding-agent/src/coordinator-mcp/policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,9 @@ function parseRootList(value: string | undefined): string[] {
}

function resolveManagedWorktreeRoot(root: string, configured: string | undefined): string {
const template = (configured?.trim() || "{repo}/.worktrees").replace(/^~(?=\/|$)/, os.homedir());
const resolved = template.replaceAll("{repo}", path.basename(root));
// Function replacers so a `$` in the home or repository path is kept verbatim.
const template = (configured?.trim() || "{repo}/.worktrees").replace(/^~(?=\/|$)/, () => os.homedir());
const resolved = template.replaceAll("{repo}", () => path.basename(root));
return path.resolve(path.isAbsolute(resolved) ? resolved : path.join(path.dirname(root), resolved));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ const GJC_VAR = "$" + "{GJC_PLUGIN_ROOT}";

export function substitutePluginRoot<T>(value: T, rootPath: string): T {
if (typeof value === "string") {
return value.replaceAll(CLAUDE_VAR, rootPath).replaceAll(GJC_VAR, rootPath) as T;
// Function replacers: a string replacement would expand `$&`, `$$`, `` $` ``
// and `$'` inside the install path instead of inserting it verbatim.
return value.replaceAll(CLAUDE_VAR, () => rootPath).replaceAll(GJC_VAR, () => rootPath) as T;
}
if (Array.isArray(value)) {
return value.map(v => substitutePluginRoot(v, rootPath)) as T;
Expand Down
3 changes: 2 additions & 1 deletion packages/coding-agent/src/gjc-runtime/launch-worktree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ export function resolveWorktreeBucketForPath(
const template = expandHomePrefix(configured || DEFAULT_WORKTREE_BUCKET, home, pathApi);
return pathApi.resolve(
pathApi.dirname(repoRoot),
template.replaceAll(REPO_NAME_PLACEHOLDER, pathApi.basename(repoRoot)),
// Function replacer so a `$` in the repository directory name is kept verbatim.
template.replaceAll(REPO_NAME_PLACEHOLDER, () => pathApi.basename(repoRoot)),
);
}

Expand Down
4 changes: 3 additions & 1 deletion packages/coding-agent/src/task/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ export function getCommand(commands: WorkflowCommand[], name: string): WorkflowC
* Replaces $@ with the provided input.
*/
export function expandCommand(command: WorkflowCommand, input: string): string {
return command.instructions.replace(/\$@/g, input);
// Function replacer: `input` is user text and must be inserted verbatim, not
// parsed for `$&`, `$$`, `` $` `` or `$'` replacement patterns.
return command.instructions.replace(/\$@/g, () => input);
}

/**
Expand Down
10 changes: 10 additions & 0 deletions packages/coding-agent/test/coordinator-mcp-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,16 @@ describe("Hermes MCP safety policy", () => {
await expect(assertCoordinatorWorkdir(config, worktree)).resolves.toBe(worktree);
});

it("derives the managed worktree root from a repository name containing replacement-pattern characters", () => {
// `{repo}` used a string replacement, so `$&` in the directory name expanded to
// the placeholder itself and `$'` spliced the template remainder into the path.
for (const name of ["a$&b", "x$$y", "q$'z"]) {
const root = path.join(os.tmpdir(), name);
const config = buildCoordinatorMcpConfig({ GJC_COORDINATOR_MCP_WORKDIR_ROOTS: root });
expect(config.managedWorktreeRoots).toEqual([path.join(root, ".worktrees")]);
}
});

it("rejects artifact symlink escapes and enforces byte caps", async () => {
const root = await tempRoot();
const outside = await tempRoot();
Expand Down
13 changes: 13 additions & 0 deletions packages/coding-agent/test/gjc-runtime/launch-worktree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,19 @@ describe("GJC_WORKTREE_DIR path red-team", () => {
});
});

describe("resolveWorktreeBucketForPath repository names with replacement-pattern characters", () => {
// `{repo}` was substituted with a string replacement, so `$&`, `$$`, `` $` `` and
// `$'` in the directory name were expanded: `a$&b` resolved to `a{repo}b`, and
// `q$'z` spliced the template remainder into the path.
for (const name of ["a$&b", "x$$y", "x$`y", "q$'z", "$1"]) {
it(`keeps ${JSON.stringify(name)} verbatim`, () => {
const repo = `/src/${name}`;
expect(resolveWorktreeBucketForPath(repo, undefined, "/home/u", path.posix)).toBe(`/src/${name}/.worktrees`);
expect(resolveWorktreeBucketForPath(repo, "~/wt/{repo}", "/home/u", path.posix)).toBe(`/home/u/wt/${name}`);
});
}
});

describe("resolveWorktreeBucketForPath Windows semantics", () => {
const home = "C:\\Users\\kim";
const repo = "C:\\repos\\app";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,14 @@ describe("substitutePluginRoot", () => {
it("returns string unchanged when no variables present", () => {
expect(substitutePluginRoot("no-vars-here", ROOT)).toBe("no-vars-here");
});
it("inserts an install path containing replacement-pattern characters verbatim", () => {
// A string replacement would expand `$&`, `$$`, `` $` `` and `$'` inside the
// path: `a$&b` became `a${CLAUDE_PLUGIN_ROOT}b`, `x$`y` lost `$`y`, and `q$'z`
// spliced the remainder of the template into the middle of the path.
for (const root of ["/plugins/a$&b", "/plugins/x$$y", "/plugins/x$`y", "/plugins/q$'z", "/plugins/$1"]) {
expect(substitutePluginRoot(`${CLAUDE_VAR}/bin/server`, root)).toBe(`${root}/bin/server`);
expect(substitutePluginRoot(`${GJC_VAR}/bin/server`, root)).toBe(`${root}/bin/server`);
expect(substitutePluginRoot([`--config=${CLAUDE_VAR}/c.json`], root)).toEqual([`--config=${root}/c.json`]);
}
});
});
23 changes: 23 additions & 0 deletions packages/coding-agent/test/task/expand-command.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { describe, expect, it } from "bun:test";
import { expandCommand, type WorkflowCommand } from "../../src/task/commands";

function command(instructions: string): WorkflowCommand {
return { name: "review", description: "", instructions, source: "user", filePath: "/tmp/review.md" };
}

describe("expandCommand", () => {
it("substitutes every $@ with the task input", () => {
expect(expandCommand(command("Review:\n$@\nThen re-check $@."), "the login fix")).toBe(
"Review:\nthe login fix\nThen re-check the login fix.",
);
});

it("inserts input containing replacement-pattern characters verbatim", () => {
// The input was passed as a string replacement, so `$&` became `$@`, `$$`
// collapsed to `$`, and `` $` `` / `$'` spliced the instructions before or
// after the placeholder into the user's text.
for (const input of ["sed s/a/$&/g", "costs $$5", "regex /^$`/", "the $' suffix", "awk '{print $1}'"]) {
expect(expandCommand(command("Task: $@ (done)"), input)).toBe(`Task: ${input} (done)`);
}
});
});
Loading