Skip to content

fix(cli): escape hostile values in generated init artifacts - #249

Open
o-mid wants to merge 5 commits into
theam:mainfrom
o-mid:fix/init-template-escaping
Open

fix(cli): escape hostile values in generated init artifacts#249
o-mid wants to merge 5 commits into
theam:mainfrom
o-mid:fix/init-template-escaping

Conversation

@o-mid

@o-mid o-mid commented Sep 1, 2026

Copy link
Copy Markdown

Fixes #228.

Summary

  • Quote workflow names in the doctor watch list so YAML-significant titles (e.g. CI: Build) parse correctly.
  • JSON-encode check allowlist entries in .claude/settings.json so shell commands with quotes stay valid JSON.
  • Escape the default branch before embedding it in protect-branch.mjs regexes (e.g. release/2026).

Mirrors the escaping in both packages/cli/src/init.mjs and packages/core/src/render.ts.

Test plan

  • pnpm --filter @theagilemonkeys/facility test
  • pnpm --filter @facility/core test
  • New hostile-input init regression test parses generated JSON/YAML/regex output

Quote doctor workflow names for YAML, JSON-encode check allowlist entries,
and escape default-branch metacharacters before they reach protect-branch
regexes. Adds regression tests for branch names, workflow titles, and
shell commands that used to break parsing silently.

@adrian-lorenzo adrian-lorenzo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

The new encoders fix the specific YAML, JSON, and regex cases, but the default branch is still interpolated directly into privileged workflow shell source. $(id) is a valid Git branch name, and generating the workflows with it produces git show "origin/$(id):..." and ?ref=$(id), both of which execute command substitution.

Please treat the branch as data in every generated workflow: encode YAML fields appropriately and pass it into shell steps through an environment variable rather than embedding it in command text. Apply the fix to both renderers and add a regression using hostile but valid refs such as $(id) and foo"bar. The import-order formatting failure also needs correcting.

Once every output context is covered, this will be a strong security fix.

adrian-lorenzo and others added 4 commits September 1, 2026 10:56
Refine the project sidebar, Product Owner composer copy, and extension-safe hydration behavior.
BREAKING CHANGE: The Facility CLI now requires Node.js 22.13.0 or Node.js 24.
Address review on theam#249: hostile but valid refs like $(id) and foo"bar
must never land in privileged run scripts. YAML contexts use quoted
scalars; shell steps read FACILITY_DEFAULT_BRANCH instead.
o-mid added a commit to o-mid/facility that referenced this pull request Sep 1, 2026
Address review on theam#249: hostile but valid refs like $(id) and foo"bar
must never land in privileged run scripts. YAML contexts use quoted
scalars; shell steps read FACILITY_DEFAULT_BRANCH instead.
@o-mid

o-mid commented Sep 1, 2026

Copy link
Copy Markdown
Author

Addressed review: default branch is now passed through FACILITY_DEFAULT_BRANCH / DEFAULT_BRANCH env vars in workflow steps instead of embedding refs in shell command text. Added DEFAULT_BRANCH_YAML for quoted YAML scalars. Regression covers $(id) and foo"bar. Fixed import order in render.ts.

@o-mid
o-mid force-pushed the fix/init-template-escaping branch from 52a3b42 to e4bc3d0 Compare September 2, 2026 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

init renders workflow names, check commands and the default branch into YAML/JSON/regex without escaping for that context

2 participants