fix: allow PostHog HogQL dollar identifiers

[mohanagy]

Fixes #169

Root cause

The PostHog command adapter rejected every $ during its generic shell-syntax preflight, before it could establish that the character was JSON data for a canonical nested PostHog read command. A legitimate $pageview query therefore became destructive and was blocked by a readonly policy.

Change

• Permit literal dollar identifiers only through the existing call <canonical-tool> <JSON object> grammar.
• Continue rejecting control characters, separators, redirections, backslashes, backticks, $() and ${...}.
• Keep non-call command grammar, canonical nested-tool validation, destructive nested-tool classification, route policy, approvals, audit, and the literal PostHog origin pin unchanged.

Regression coverage

• Unit: $pageview + math: dau now classifies as read; $() / ${...}, $ in search/tool names, and destructive nested calls remain blocked.
• Policy provenance: the trusted adapter reports high-confidence read.
• End-to-end: a readonly Miftah server previews and forwards the literal official PostHog exec call, while write calls remain denied.

Validation

• Focused regression initially failed before the fix: expected read, received destructive.
• npx vitest run tests/posthog-command-wrapper.test.ts tests/risk-classifier.test.ts tests/mcp-wrapper.test.ts -t "PostHog|classifies canonical PostHog command-wrapper reads without weakening command safety"
• npm test — 85 files passed, 1 skipped; 1,079 tests passed, 23 skipped.
• npm run lint
• npm run typecheck
• git diff --check

Independent security review found no bypass after the JSON-payload scoping refinement.

Summary by CodeRabbit

• Bug Fixes
  • Improved command safety checks for JSON-based calls.
  • Valid dollar-prefixed identifiers, such as $pageview, are now recognized as read-only where appropriate.
  • Unsafe shell-style dollar substitutions, including $(...) and ${...}, are now blocked in command payloads.
  • Updated risk classification for analytics queries, destructive commands, malformed inputs, and command previews.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6c8b293e-dcf1-40d2-97c6-dc5946fd5232

📥 Commits

Reviewing files that changed from the base of the PR and between 8fc43aa and 5a4bf0a.

📒 Files selected for processing (4)

• src/policy/posthog-command-wrapper.ts
• tests/mcp-wrapper.test.ts
• tests/posthog-command-wrapper.test.ts
• tests/risk-classifier.test.ts

---

📝 Walkthrough

Walkthrough

PostHog command risk parsing now permits literal HogQL dollar identifiers while separately rejecting $(...) and ${...} substitutions in validated JSON call payloads. Tests cover classification, wrapper previews, audit evidence, and destructive command cases.

Changes

PostHog dollar safety

Layer / File(s)	Summary
Separate dollar identifier and substitution validation src/policy/posthog-command-wrapper.ts	Generic unsafe syntax checks no longer reject every dollar sign; validated call payloads are separately checked for $(...) and ${...} substitution patterns.
Validate classification and wrapper behavior tests/posthog-command-wrapper.test.ts, tests/risk-classifier.test.ts, tests/mcp-wrapper.test.ts	Tests classify valid HogQL dollar identifiers as read-only, retain destructive results for unsafe patterns, and update wrapper route-preview and audit assertions for JSON call commands.

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

Possibly related PRs

• mohanagy/miftah#109: Also modifies PostHog command-wrapper risk classification and trusted-adapter behavior.
• mohanagy/miftah#146: Also changes PostHog risk classification and trusted command metadata.

Poem

A bunny found $pageview bright,
And kept its query safe and right.
$(whoami) was turned away,
${HOME} could not sneak in to play.
Hops for trusted reads today!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description has useful detail, but it misses the required Summary and Security impact sections.	Add the missing Summary and Security impact sections, and align Validation with the repository template's checklist.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title succinctly describes the main fix: permitting PostHog HogQL dollar identifiers.
Linked Issues check	✅ Passed	The changes implement the scoped PostHog fix, keep unsafe shell substitutions blocked, and add matching regression coverage.
Out of Scope Changes check	✅ Passed	The diff stays focused on PostHog risk classification and tests, with no obvious unrelated code changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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 fix/169-posthog-hogql-dollar

---

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