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
116 changes: 116 additions & 0 deletions src/patches/systemPrompts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ vi.mock('../systemPromptSync', async () => {
return {
...actual,
loadSystemPromptsWithRegex: vi.fn(),
loadIdentifierMapUnion: vi.fn(),
};
});

Expand Down Expand Up @@ -92,6 +93,9 @@ function setupMocks(
describe('systemPrompts.ts', () => {
beforeEach(() => {
vi.clearAllMocks();
// Default: empty union (no known human-names) → guard never skips.
// Tests exercising the guard set their own union explicitly.
vi.mocked(promptSync.loadIdentifierMapUnion).mockResolvedValue(new Set());
});

describe('applySystemPrompts', () => {
Expand Down Expand Up @@ -311,6 +315,118 @@ describe('systemPrompts.ts', () => {
spy.mockRestore();
});

it('should skip when a single-word human-name placeholder (no underscore) leaks, via the identifierMap union', async () => {
// ${VERSION} has no underscore, so the old ALL_CAPS_WITH_UNDERSCORE
// grammar regex missed it. The union check catches it because VERSION is
// a member of the leaf's identifierMap vocabulary.
vi.mocked(promptSync.loadIdentifierMapUnion).mockResolvedValue(
new Set(['VERSION'])
);
const mockPromptData = buildMockPromptData({
prompt: { content: 'before ${VERSION} after' },
regex: 'before \\$\\{VERSION\\} after',
getInterpolatedContent: () => 'before ${VERSION} after',
pieces: ['before ${VERSION} after'],
});

setupMocks(mockPromptData);

const cliContent = 'desc:`before ${VERSION} after`';

const result = await applySystemPrompts(cliContent, '1.0.0', false);

expect(result.newContent).toBe(cliContent);
expect(result.results[0].applied).toBe(false);
expect(result.results[0].details).toContain('unresolved placeholder');
});

it('should skip prompt when an unescaped human-name placeholder leaks into a backtick literal', async () => {
// The markdown references ${STALE_VAR_NAME} -- a human-name in the leaf's
// identifierMap union but with no entry in THIS version's prompt data, so
// getInterpolatedContent leaves it verbatim. Embedding it into a `${...}`
// template-literal slot would ReferenceError at launch, so the guard skips
// the prompt and keeps CC's original blob.
vi.mocked(promptSync.loadIdentifierMapUnion).mockResolvedValue(
new Set(['STALE_VAR_NAME'])
);
const mockPromptData = buildMockPromptData({
prompt: { content: 'before ${STALE_VAR_NAME} after' },
regex: 'before \\$\\{STALE_VAR_NAME\\} after',
getInterpolatedContent: () => 'before ${STALE_VAR_NAME} after',
pieces: ['before ${STALE_VAR_NAME} after'],
});

setupMocks(mockPromptData);

const cliContent = 'desc:`before ${STALE_VAR_NAME} after`';

const result = await applySystemPrompts(cliContent, '1.0.0', false);

expect(result.newContent).toBe(cliContent);
expect(result.results[0].applied).toBe(false);
expect(result.results[0].details).toContain('unresolved placeholder');
});

it('should NOT skip an underscored ${NAME} that is not a human-name (real runtime binding, absent from the union)', async () => {
// The old grammar regex flagged any ALL_CAPS_WITH_UNDERSCORE token,
// false-positiving on a genuine runtime interpolation. The union check
// only skips known human-names, so a real binding absent from the union
// is applied normally.
vi.mocked(promptSync.loadIdentifierMapUnion).mockResolvedValue(
new Set(['GLOB_TOOL_NAME'])
);
const mockPromptData = buildMockPromptData({
prompt: { content: 'wait ${MAX_RETRY_COUNT} ms then stop' },
regex: 'wait \\$\\{MAX_RETRY_COUNT\\} ms then go',
getInterpolatedContent: () => 'wait ${MAX_RETRY_COUNT} ms then stop',
pieces: ['wait ${MAX_RETRY_COUNT} ms then go'],
});

setupMocks(mockPromptData);

const cliContent = 'desc:`wait ${MAX_RETRY_COUNT} ms then go`';

const result = await applySystemPrompts(cliContent, '1.0.0', false);

expect(result.results[0].applied).toBe(true);
expect(result.results[0].details ?? '').not.toContain(
'unresolved placeholder'
);
expect(result.newContent).toBe(
'desc:`wait ${MAX_RETRY_COUNT} ms then stop`'
);
});

it('should NOT skip when an ALL_CAPS placeholder is backslash-escaped (literal env-var docs)', async () => {
// `\${CLAUDE_PLUGIN_ROOT}`-style tokens are intentional literal text, not
// interpolation slots. Even when the name IS a union member, the
// backslash-escape (negative lookbehind) keeps the guard from flagging it
// -- so the escape, not mere absence from the union, is what protects it.
vi.mocked(promptSync.loadIdentifierMapUnion).mockResolvedValue(
new Set(['CLAUDE_PLUGIN_ROOT'])
);
const mockPromptData = buildMockPromptData({
prompt: { content: 'use \\${CLAUDE_PLUGIN_ROOT} now' },
regex: 'use \\\\\\$\\{CLAUDE_PLUGIN_ROOT\\} here',
getInterpolatedContent: () => 'use \\${CLAUDE_PLUGIN_ROOT} now',
pieces: ['use \\${CLAUDE_PLUGIN_ROOT} here'],
});

setupMocks(mockPromptData);

const cliContent = 'desc:`use \\${CLAUDE_PLUGIN_ROOT} here`';

const result = await applySystemPrompts(cliContent, '1.0.0', false);

// Guard did NOT skip it: the escaped token is treated as literal text and
// the override is applied (here, swapping "here" -> "now").
expect(result.results[0].applied).toBe(true);
expect(result.results[0].details ?? '').not.toContain(
'unresolved placeholder'
);
expect(result.newContent).toBe('desc:`use \\${CLAUDE_PLUGIN_ROOT} now`');
});

it('should auto-escape multiple backticks in template literal context', async () => {
const mockPromptData = buildMockPromptData({
content: 'Use `foo` and `bar` for config',
Expand Down
56 changes: 56 additions & 0 deletions src/patches/systemPrompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
loadSystemPromptsWithRegex,
reconstructContentFromPieces,
escapeDepthZeroBackticks,
loadIdentifierMapUnion,
} from '../systemPromptSync';
import { setAppliedHashes, computeMD5Hash } from '../systemPromptHashIndex';
import { findAllMatchesWithStackFallback } from '../safeRegexMatch';
Expand Down Expand Up @@ -98,6 +99,12 @@ export const applySystemPrompts = async (
);
debug(`Loaded ${systemPrompts.length} system prompts with regexes`);

// The set of every tweakcc human-name the leaf has ever used as a
// placeholder, unioned across all bundled prompt JSONs. Used below to detect
// a leaked (unsubstituted) human-name surviving into a backtick template
// literal. Loaded once per apply.
const identifierMapUnion = await loadIdentifierMapUnion();

// Track per-prompt results
const results: PatchResult[] = [];
const appliedHashUpdates: Record<string, string> = {};
Expand Down Expand Up @@ -167,6 +174,55 @@ export const applySystemPrompts = async (
const matchIndex = match.index;
const delimiter = matchIndex > 0 ? content[matchIndex - 1] : '';

// Guard: a tweakcc human-name placeholder that survives interpolation into
// a `${...}` template-literal slot is invalid JS and ReferenceErrors at
// launch (or when the prompt's code path first runs). This happens when the
// prompt-data identifierMap vocabulary changed between CC versions (e.g.
// PROMPT_VAR_N -> *_TOOL_NAME at 2.1.168, or a renamed semantic name like
// OPTIONAL_TAIL_NOTE) while the markdown still references the old name, so
// applyIdentifierMapping finds nothing to substitute and leaves the
// placeholder verbatim. Detect a surviving `${NAME}` whose NAME is a member
// of the identifierMap union (the set of every human-name the leaf has ever
// used as a placeholder) and that appears unchanged in BOTH the markdown
// source and the interpolated output. Validating against the union -- rather
// than guessing an ALL_CAPS_WITH_UNDERSCORE grammar -- catches single-word
// names like ${VERSION} the grammar missed and never false-positives on real
// minified vars (e.g. `${HL7}`), which are never human-names. Only dangerous
// inside backtick template literals; the same token in a plain '...'/"..."
// string is inert. Skip the prompt and keep CC's original blob rather than
// shipping a binary that won't boot.
if (delimiter === '`') {
// Only UNescaped `${NAME}` is dangerous: a backslash-escaped
// `\${NAME}` is intentional literal text (e.g. the env-var docs
// `\${CLAUDE_PLUGIN_ROOT}` in the cowork plugin prompts, which have an
// empty identifierMap) and survives into the template literal verbatim.
// The negative lookbehind excludes those so they aren't false-flagged.
const placeholderRe = /(?<!\\)\$\{([A-Za-z_][A-Za-z0-9_]*)\}/g;
const inOutput = new Set(
[...interpolatedContent.matchAll(placeholderRe)].map(m => m[1])
);
const leaked = [...inOutput].find(
name =>
identifierMapUnion.has(name) &&
new RegExp('(?<!\\\\)\\$\\{' + name + '\\}').test(prompt.content)
);
if (leaked) {
console.log(
chalk.red(
`Unresolved placeholder \${${leaked}} in "${prompt.name}" (markdown vocabulary out of sync with CC ${version} prompt data) - skipping`
)
);
results.push({
id: promptId,
name: prompt.name,
group: PatchGroup.SYSTEM_PROMPTS,
applied: false,
details: `unresolved placeholder \${${leaked}} - markdown out of sync with prompt data`,
});
continue;
}
}

// Calculate character counts for this prompt (both with human-readable placeholders)
// Note: trim() to match how markdown files are parsed and how whitespace is applied
const originalBaselineContent = reconstructContentFromPieces(
Expand Down
2 changes: 1 addition & 1 deletion src/systemPromptDownload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { PROMPT_CACHE_DIR } from './config';
// publishes one) skip the network fetch when run via `node dist/index.mjs`.
// Published npm builds strip data/ via .npmignore, so this returns null
// for those installs and the network path takes over.
function findRepoPromptsDir(): string | null {
export function findRepoPromptsDir(): string | null {
try {
const here = fileURLToPath(import.meta.url);
let dir = path.dirname(here);
Expand Down
35 changes: 35 additions & 0 deletions src/systemPromptSync.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { describe, it, expect, afterEach } from 'vitest';
import {
loadIdentifierMapUnion,
clearIdentifierMapUnionCache,
} from './systemPromptSync';

describe('systemPromptSync.ts', () => {
describe('loadIdentifierMapUnion', () => {
afterEach(() => {
clearIdentifierMapUnionCache();
});

it('unions identifierMap values across the bundled data/prompts/*.json', async () => {
const union = await loadIdentifierMapUnion();

// Reads the repo-local bundled prompt data in a checkout.
expect(union.size).toBeGreaterThan(0);
// A human-name present in ~all 2.1.x prompt files.
expect(union.has('GLOB_TOOL_NAME')).toBe(true);
// Real minified vars are never human-names, so never in the union.
expect(union.has('HL7')).toBe(false);
});

it('caches the union and rebuilds it after the cache is cleared', async () => {
const first = await loadIdentifierMapUnion();
const second = await loadIdentifierMapUnion();
expect(second).toBe(first); // same instance while cached

clearIdentifierMapUnionCache();
const third = await loadIdentifierMapUnion();
expect(third).not.toBe(first); // fresh instance after clear
expect(third).toEqual(first); // same contents
});
});
});
60 changes: 59 additions & 1 deletion src/systemPromptSync.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import * as fs from 'node:fs/promises';
import * as path from 'path';
import matter from 'gray-matter';
import { downloadStringsFile } from './systemPromptDownload';
import {
downloadStringsFile,
findRepoPromptsDir,
} from './systemPromptDownload';
import {
storeHashes,
getPromptHash,
Expand Down Expand Up @@ -60,6 +63,61 @@ export const clearShadowSetCache = (): void => {
shadowSetCache = null;
};

/**
* Union of every identifierMap value across the leaf's bundled
* data/prompts/prompts-*.json — the complete set of tweakcc human-names the
* fork has ever used as placeholders. The apply-time unresolved-placeholder
* guard (applySystemPrompts) uses this as its detector: a `${TOKEN}` that
* survives interpolation into a backtick template literal is a leaked
* human-name (skip the prompt, keep CC's blob) iff TOKEN is a member of this
* union. A leaked name is stale precisely because it is absent from the
* CURRENT version's identifierMap, so the guard must validate against the
* cross-version union — not just the version being patched. Membership also
* makes the detector immune to real minified vars (e.g. `HL7`), which are
* never human-names and so never appear in the union.
*
* Empty when the bundled dir is absent (npm-installed builds strip data/ via
* .npmignore): the guard then degrades to never skipping, matching pre-guard
* behavior for those installs.
*
* Cached after first call; reset by clearIdentifierMapUnionCache.
*/
let identifierMapUnionCache: Set<string> | null = null;
export const loadIdentifierMapUnion = async (): Promise<Set<string>> => {
if (identifierMapUnionCache) return identifierMapUnionCache;
const union = new Set<string>();
const dir = findRepoPromptsDir();
if (dir) {
let files: string[];
try {
files = await fs.readdir(dir);
} catch {
files = [];
}
for (const name of files) {
if (!name.startsWith('prompts-') || !name.endsWith('.json')) continue;
try {
const text = await fs.readFile(path.join(dir, name), 'utf8');
const parsed = JSON.parse(text) as StringsFile;
for (const prompt of parsed.prompts ?? []) {
for (const value of Object.values(prompt.identifierMap ?? {})) {
if (value) union.add(value);
}
}
} catch {
// Skip unreadable/malformed files — a partial union still guards the
// names it could load.
continue;
}
}
}
identifierMapUnionCache = union;
return union;
};
export const clearIdentifierMapUnionCache = (): void => {
identifierMapUnionCache = null;
};

/**
* Prompt structure from strings-X.Y.Z.json files
*/
Expand Down