fix(create-bestax): reject dot-only project names, pin icon versions, bundle bestax-icons skill

[allxsmith]

Pull Request

Description

Fixes from a full security review of the repo (scaffolder, workflows, composite actions, repo docs). The written deep-dive report is delivered separately (not committed — it discloses details best kept out of the public repo until this merges).

• create-bestax (create-bestax)
• Other (please specify): GitHub Actions workflows, .github/actions/verified-commit, CODEOWNERS, SECURITY.md

create-bestax (release-affecting — the squash commit title above cuts one patch release):

• Destructive path bug (high severity): . and .. passed project-name validation, resolved to the current/parent directory, and --yes then emptied that directory — npm create bestax .. -- -y wiped the parent dir on an end-user machine. All leading-dot names are now rejected (slashes were already blocked). Trade-off: npm create bestax . (scaffold into the current dir) is now rejected too, with a message saying to pass a directory name.
• Floating latest deps: generated apps pinned icon libraries to latest; they now get the caret ranges bulma-ui/docs are tested against (@fortawesome/fontawesome-free ^7.2.0, @mdi/font ^7.4.47, material-icons ^1.13.14, material-symbols ^0.45.2), with a drift-guard test. The pinned ionicons CDN tags now carry a comment documenting why SRI is deliberately omitted (the ESM loader dynamically imports per-icon chunks that can't carry integrity hashes).
• Skill drift: the generated CLAUDE.md advertises bestax-icons, but sync-skills.mjs never bundled it (omission from feat(create-bestax): bestax-icons skill — teach agents the icon system #302). It now ships.

Workflow hardening (release-neutral ci:/chore:/docs: commits riding along):

• test-deploy.yml: github.head_ref (attacker-chosen PR-branch text) was interpolated into the wrangler command; it's now sanitized to [A-Za-z0-9-] via a step output first.
• verified-commit action: inputs were interpolated directly into the github-script body and an execSync(`find …`) string — a latent injection sink for any future caller. Inputs now arrive via env: and find runs through execFileSync with an argument array. Behavior unchanged for the current caller (visual-regression.yml).
• claude.yml: added the same authoritative live collaborator role re-check its siblings use, plus persist-credentials: false. Behavior change: read-only COLLABORATOR/MEMBER accounts can no longer trigger @claude — intentional, matches bestaxbot-reply.yml/claude-implement.yml.
• claude.yml, claude-implement.yml, claude-review.yml, ci.yml: moved to the repo's own permissions: {} + per-job pattern; CI's PR-path jobs drop the unused contents: write/pull-requests: write and add persist-credentials: false. The publish job is untouched.
• New .github/CODEOWNERS (/.github/ @allxsmith) — inert until "Require review from Code Owners" is enabled in the main branch protection.

Related Issue(s)

Related to #302 (bestax-icons skill was added there but never bundled into create-bestax).

Type of Change

• Bug fix
• Other (please describe): security hardening of CI/workflows; SECURITY.md doc update

Checklist

• My code follows the project style guidelines
• I have performed a self-review of my code
• I have added tests that prove my fix is effective or that my feature works
• I have added/updated documentation as needed
• I have updated Storybook stories as needed (bulma-ui) — n/a, no bulma-ui changes
• My changes require a change to the documentation — n/a beyond SECURITY.md
• All new and existing tests passed
• Any relevant dependencies are updated
• If this PR changes commands, conventions, or package structure, the affected CLAUDE.md files are updated — n/a, no command/convention changes

Screenshots / Demos

Live smoke test after the fix (scratchpad, built CLI):

$ node create-bestax/dist/index.js .. -y
✖ Project name cannot start with a dot (names like "." or ".." would scaffold outside a new directory) — pass a directory name
✖ Operation cancelled   (exit 1, parent directory untouched)

Generated package.json now contains "@mdi/font": "^7.4.47" (no latest), and .claude/skills/ includes bestax-icons.

Additional Context

Verified locally: create-bestax jest suite 191/191 passing with coverage 97.9% stmts / 87.9% branches (thresholds 95/78); lint, typecheck, format:check, gen:catalog:check, check:conformance all green; every edited workflow YAML parses. This PR touches .github/**, so the AI loop won't drive it — it needs a human review and squash-merge. The squash-commit title should stay fix(create-bestax): … so exactly one create-bestax patch releases.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Hk9z4hXDifYFQg9VLjxEo3

---

Generated by Claude Code

Summary by CodeRabbit

• New Features

  • Added the Bestax Icons skill to generated projects.
  • Added pinned versions for supported icon libraries in generated package.json files.
  • Added clearer guidance when project names begin with a dot.

• Bug Fixes

  • Prevented scaffolding projects with dot-prefixed names.
  • Improved preview deployment branch-name handling.
  • Added permission checks before automated workflows run.

• Security

  • Restricted workflow permissions to the jobs that require them.
  • Improved secure file discovery and disabled credential persistence during checkout.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5550844f-190e-4510-a145-129656c7054d

📥 Commits

Reviewing files that changed from the base of the PR and between 4b2b797 and 5c661d2.

📒 Files selected for processing (13)

• .github/CODEOWNERS
• .github/actions/verified-commit/action.yml
• .github/workflows/claude-implement.yml
• .github/workflows/claude-review.yml
• .github/workflows/claude.yml
• .github/workflows/test-deploy.yml
• create-bestax/scripts/sync-skills.mjs
• create-bestax/src/__tests__/constants.test.ts
• create-bestax/src/__tests__/project-creator.test.ts
• create-bestax/src/__tests__/validators.test.ts
• create-bestax/src/constants.ts
• create-bestax/src/project-creator.ts
• create-bestax/src/validators.ts

🚧 Files skipped from review as they are similar to previous changes (12)

• .github/CODEOWNERS
• create-bestax/src/tests/constants.test.ts
• .github/workflows/test-deploy.yml
• create-bestax/src/tests/validators.test.ts
• .github/workflows/claude.yml
• .github/workflows/claude-implement.yml
• .github/workflows/claude-review.yml
• create-bestax/src/validators.ts
• create-bestax/src/project-creator.ts
• .github/actions/verified-commit/action.yml
• create-bestax/src/tests/project-creator.test.ts
• create-bestax/src/constants.ts

---

Walkthrough

Changes

GitHub workflow security

Layer / File(s)	Summary
Verified commit input handling .github/actions/verified-commit/action.yml	Action inputs are passed through environment variables, file discovery uses argument-safe execution, and commit creation reads the environment-sourced message.
Workflow permission scoping .github/CODEOWNERS, .github/workflows/claude-implement.yml, .github/workflows/claude-review.yml, .github/workflows/claude.yml	Workflows use default-deny permissions with required scopes declared at job level, and .github/ ownership is documented.
Claude trigger authorization .github/workflows/claude.yml	The workflow checks collaborator roles before checkout, setup, and Claude execution, while disabling persisted checkout credentials.
Preview branch sanitization .github/workflows/test-deploy.yml	Preview branch names are normalized to allowed characters, limited to 63 characters, and passed to the Pages deployment command.

create-bestax scaffold consistency

Layer / File(s)	Summary
Project and icon metadata contracts create-bestax/src/constants.ts, create-bestax/src/validators.ts	Leading-dot project names are rejected, and icon libraries define explicit package version metadata.
Generated dependency and skill wiring create-bestax/src/project-creator.ts, create-bestax/scripts/sync-skills.mjs	Generated dependencies use configured icon versions, ionicons behavior is documented, and bestax-icons is synchronized.
Scaffold behavior tests create-bestax/src/__tests__/*	Tests cover leading-dot names and pinned icon dependency versions.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant GitHubAPI
  participant Repository
  participant Claude
  GitHubActions->>GitHubAPI: Check triggering collaborator role
  GitHubAPI-->>GitHubActions: Return role
  GitHubActions->>Repository: Checkout when role is allowed
  Repository->>Claude: Provide checked-out workspace
  Claude->>GitHubActions: Execute workflow actions

Loading

Possibly related PRs

• allxsmith/bestax#260: Updates Claude workflow controls for write and edit permissions.
• allxsmith/bestax#302: Adds the bestax-icons skill content consumed by the synchronization script.

Suggested labels: dependencies

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately captures the main create-bestax fixes and the bundled bestax-icons update.
Description check	✅ Passed	The description follows the template well, covering summary, affected packages, issue, change type, checklist, demo, and context.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/bestax-security-review-4j1549

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages.

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Preview Deployment

Preview URL: https://b4faaa82.bestax.pages.dev

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

create-bestax/src/project-creator.ts (1)

268-269: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Do not retain latest as a fallback for package-backed libraries.

library.packageVersion ?? 'latest' still permits an icon entry with packageName but missing version metadata to generate a floating dependency, bypassing the new contract in constants.ts. Require packageVersion whenever packageName is present, or fail fast before writing package.json; reserve missing versions for CDN/no-package entries. This follows the scaffold’s explicit non-floating dependency contract.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@create-bestax/src/project-creator.ts` around lines 268 - 269, Update the
dependency generation logic around packageJson.dependencies so package-backed
libraries require a defined library.packageVersion and fail fast before writing
package.json when it is missing; only allow absent versions for CDN/no-package
entries, and remove the 'latest' fallback.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/actions/verified-commit/action.yml:
- Around line 20-43: Update the file-discovery logic around the script’s pattern
and basePath handling to derive the filename glob passed to find from
process.env.FILE_PATTERN instead of hardcoding '*.png'. Preserve the existing
recursive base-directory behavior and pass the derived glob as find’s -name
argument so extensions supported by the documented file-pattern input are
discovered.

---

Nitpick comments:
In `@create-bestax/src/project-creator.ts`:
- Around line 268-269: Update the dependency generation logic around
packageJson.dependencies so package-backed libraries require a defined
library.packageVersion and fail fast before writing package.json when it is
missing; only allow absent versions for CDN/no-package entries, and remove the
'latest' fallback.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 31d91fd7-590d-4659-82fc-842f05cfc133

📥 Commits

Reviewing files that changed from the base of the PR and between 74e3d4e and 65eec75.

📒 Files selected for processing (15)

• .github/CODEOWNERS
• .github/actions/verified-commit/action.yml
• .github/workflows/ci.yml
• .github/workflows/claude-implement.yml
• .github/workflows/claude-review.yml
• .github/workflows/claude.yml
• .github/workflows/test-deploy.yml
• SECURITY.md
• create-bestax/scripts/sync-skills.mjs
• create-bestax/src/__tests__/constants.test.ts
• create-bestax/src/__tests__/project-creator.test.ts
• create-bestax/src/__tests__/validators.test.ts
• create-bestax/src/constants.ts
• create-bestax/src/project-creator.ts
• create-bestax/src/validators.ts

[github-actions]

Preview Deployment

Preview URL: https://bc600975.bestax.pages.dev

[allxsmith]

Deep review — 1 finding + 1 merge blocker

#	Severity	Area	Finding	Location
1	🟡 Minor	Security (defense-in-depth)	find argv injection via a leading-dash basePath: a future caller passing file-pattern: "-delete" would make GNU find default the path to . and evaluate -delete as an expression — deleting the workspace. -name's argument is safe (it consumes the next argv slot), but the first positional arg isn't. Prefix it: execFileSync('find', ['./' + basePath.replace(/^\.\//, ''), ...]) or reject leading-dash patterns. Same "latent sink for any future caller" rationale this PR itself applies to the template-interpolation fix.	.github/actions/verified-commit/action.yml
—	🔴 Blocker	Merge state	The branch is now CONFLICTING with main: #313 rewrote SECURITY.md (its latest-only policy + supply-chain section supersedes this PR's table fix — drop this PR's SECURITY.md hunk in the rebase) and #314 restructured ci.yml permissions (top-level contents: read vs this PR's permissions: {} + per-job — both are least-privilege; pick one style during the rebase).	SECURITY.md, .github/workflows/ci.yml

Verified sound (the load-bearing claims):

• Dot-name rejection (validators.ts): the check runs before the regex (so ../ gets the dot message, not the char message), catches ., .., .hidden; /, \, ~ were already excluded by the regex. Tests cover validateProjectName, isValidProjectName, and the getProjectName CLI path. The documented trade-off (rejecting npm create bestax .) is real but the right call given --yes emptied the target.
• Icon-version pinning: caret ranges match what bulma-ui/docs test against; the drift-guard test (packageVersion must match ^\^\d+\.\d+\.\d+$, never latest) closes the regression door. Nit: library.packageVersion ?? 'latest' in project-creator.ts re-opens it at runtime for an entry the test somehow misses — consider throwing instead of falling back.
• verified-commit env-var hardening: inputs now reach the script only via env:; execFileSync with an argument array kills the backtick/quote injection. Correct.
• test-deploy sanitization: printf '%s' | tr -cs 'a-zA-Z0-9-' '-' | cut -c1-63 — no trailing-newline artifact, complement+squeeze collapses runs, output is safe to interpolate by construction. Correct.
• claude.yml live role re-check: matches the sibling workflows' pattern; gh api … || echo none fails closed for non-collaborators (404 → none → skip); every subsequent step is gated on the output. The documented behavior change (read-only COLLABORATOR loses @claude) is intentional.
• Per-job permissions: with top-level permissions: {}, every job in the touched workflows declares its own block, and ci.yml's publish job keeps its pre-existing write scopes — nothing silently lost.
• bestax-icons in sync-skills: matches the skills the generated CLAUDE.md advertises.

Overall: the substantive security fixes are correct and well-tested; the destructive-path fix in particular is exactly right. Address the rebase (blocker), and optionally the two hardening nits, and this is ready.

[allxsmith]

Both review items addressed: the branch is rebased onto current main (the SECURITY.md commit dropped — #313's merged rewrite supersedes it; ci.yml resolved to main's merged #314 treatment, keeping this PR's claude-implement/claude-review per-job permission moves), and the find leading-dash guard landed in 5c661d2. All commits signed.

[github-actions]

Preview Deployment

Preview URL: https://7e36206a.bestax.pages.dev

[github-actions]

🎉 This PR is included in version 3.3.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 5.7.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[bestax-release-bot]

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀