Security and consistency audit findings

[xiaolai]

Overview

Ran a systematic audit of the impeccable repo covering security, NL artifact quality, and cross-component consistency. This issue documents all findings across severity tiers, with PRs submitted for the actionable High and Medium items.

Security Findings

High: new Function() code evaluation in build scripts (3 sites)

PR: #92

Three build scripts extract the ANTIPATTERNS array from src/detect-antipatterns.mjs via regex, then evaluate it with new Function():

File	Line	Context
scripts/build.js	122	validateAntipatternRules() -- runs during bun run build
scripts/build-extension.js	63	antipatterns.json extraction -- runs during extension build
scripts/lib/sub-pages-data.js	107	readAntipatternRules() -- runs at dev-server boot and during build

Impact: Any commit modifying the regex-matched region in detect-antipatterns.mjs gains arbitrary code execution at build time. The sub-pages-data.js instance also runs at dev-server startup (bun run dev), not just during CI.

Fix: ANTIPATTERNS is already exported from the source module (line 3577). The three scripts can import it directly via ESM, eliminating the regex+eval path entirely.

Medium: Incomplete selector escaping in devtools panel

PR: #93

extension/devtools/panel.js line 503-511: The inspectElement() function passes CSS selectors into chrome.devtools.inspectedWindow.eval() with manual escaping that only handles backslash and single-quote characters.

Selectors containing backticks, newlines, null bytes, or Unicode escape sequences could break out of the string literal and inject JS into the inspected page context. The selector strings originate from detector output, so exploitation requires a crafted page that produces a malicious selector value.

Fix: Replace manual escaping with JSON.stringify(), which handles all special characters.

Medium: execSync with shell-interpolated paths (Low risk, no PR)

bin/commands/skills.mjs line 450 shell-interpolates two tmpdir-derived paths into an unzip command. Both are derived from os.tmpdir() + Date.now() (not user input), so exploitation requires prior write access to /tmp. Very low probability but avoidable with Node's built-in child_process.execFile (no shell) or a native unzip library.

Low: Substring-based path traversal guard in dev server

server/index.js line 195 uses url.pathname.includes('..') which is a substring check rather than a path.resolve() + prefix assertion. In practice, Bun's new URL() normalizes the pathname and file().size returns 0 for missing paths, so no traversal is currently possible. The server also runs in development mode only. Flagged as a hygiene issue.

Not an issue: Unpinned semver ranges in package.json

Initially flagged as Medium, but bun.lock exists in the repo, which pins all dependency versions for reproducible builds. The caret ranges in package.json are standard practice when a lockfile is present. False positive.

NL Artifact Quality Scores

Artifact	Score	Notes
.claude-plugin/plugin.json	100/100	All required fields present, valid semver
.claude-plugin/marketplace.json	100/100	Clean
CLAUDE.md	80/100	-10: stale evals/AGENT.md reference (gitignored, absent from working tree); -5: no prerequisites section; -5: large descriptive evals section about inaccessible private framework

Aggregate: 93/100

CLAUDE.md recommendations

1. The Evals Framework section (42 lines) references evals/AGENT.md at lines 101 and 138, but the entire evals/ directory is gitignored and absent. Any contributor following "read evals/AGENT.md first" hits a dead end. Consider collapsing this section to a single line: "Evals are private and gitignored. See evals/AGENT.md if you have access."
2. No prerequisites section listing required tool versions (bun, node >= 18 per package.json engines field).

Cross-Component Consistency Issues

Broken references (3)

1. .claude/agents/anti-patterns.md references source/skills/impeccable/SKILL.md (5 times). The source/ directory is the authoring tree, not present in the installed plugin. An agent working from the installed copy will fail to find this file.

2. .claude/skills/arrange/SKILL.md line 49 references reference/spatial-design.md as a relative path, saying it is "from the impeccable skill." The relative path resolves to .claude/skills/arrange/reference/spatial-design.md, which does not exist. The actual file is at .claude/skills/impeccable/reference/spatial-design.md.

3. .claude/skills/typeset/SKILL.md line 49 has the same issue with reference/typography.md.

Stale command count (contradiction)

plugin.json, marketplace.json, and AGENTS.md all say "18 commands", but there are 21 non-deprecated user-invocable skills in .claude/skills/. The CLAUDE.md "Adding New Skills" section explicitly lists these files as places to update the count, suggesting the update step was missed during recent skill additions.

Note: The build system already has a generateCounts() function that validates these numbers at build time (scripts/build.js line 29). This validator should be catching the stale "18" -- worth checking if it is running correctly or if the pattern match is not matching the count format in these files.

Deprecated skill with conflicting signals

.claude/skills/teach-impeccable/SKILL.md has user-invocable: true in frontmatter (makes it discoverable as a command), but the body says "Do NOT proceed with any teach flow here" and redirects to /impeccable teach. The user-invocable flag should be false or the skill should be removed.

Same issue with .claude/skills/frontend-design/SKILL.md (deprecated redirect to /impeccable).

Undocumented .agents/skills/ tree

The .agents/skills/ directory (23 files) mirrors .claude/skills/ exactly. Neither CLAUDE.md, AGENTS.md, nor any skill body references this tree by path. It appears to be a build output for VS Code Copilot / Antigravity (based on the build script's PROVIDERS config), but this is not documented. External contributors may not realize it is generated and attempt to edit it directly.

Terminology drift

• "commands" vs "skills": plugin.json and marketplace.json call them "commands" in descriptions; the filesystem and CLAUDE.md call them "skills". No explanation of the distinction.
• "Color and Theme" vs "Color and Contrast": The impeccable SKILL.md uses "Color and Theme" as a heading; the anti-patterns agent normalizes it to "Color and Contrast". The mapping is only documented in the agent, not the skill.
• Hardcoded "25 patterns" in critique/SKILL.md: if rules are added or removed, this number drifts silently.

Summary Table

#	Severity	Category	Finding	Status
1	High	Security	new Function() eval in 3 build scripts	PR #92
2	Medium	Security	Incomplete selector escaping in devtools eval()	PR #93
3	Medium	Security	execSync with shell-interpolated paths	Documented (low risk)
4	Low	Security	Substring path traversal guard in dev server	Documented
5	--	Security	Unpinned semver ranges	False positive (lockfile exists)
6	Medium	Quality	CLAUDE.md references gitignored evals/AGENT.md	Documented
7	Low	Quality	CLAUDE.md missing prerequisites section	Documented
8	Medium	Consistency	Stale "18 commands" count (actual: 21)	Documented
9	Medium	Consistency	3 broken relative path references in skills	Documented
10	Low	Consistency	Deprecated skills still marked user-invocable	Documented
11	Low	Consistency	.agents/skills/ tree undocumented	Documented
12	Low	Consistency	Terminology drift (commands/skills, section names)	Documented

---

Audit performed with nlpm (security-scanner + scorer + checker agents).