docs(cli): refresh the bundled stash-cli skill; fix the post-cutover read path and the .env.example guard#594
docs(cli): refresh the bundled stash-cli skill; fix the post-cutover read path and the .env.example guard#594coderdan wants to merge 3 commits into
Conversation
…I surface The `stash-cli` skill ships inside the `stash` tarball (`dist/skills/`) and is copied into the user's `.claude/skills/` / `.codex/skills/`, or inlined into their `AGENTS.md`, at handoff time. A stale skill becomes stale guidance in the user's project, so treat it as a shipped artifact rather than internal docs. Rewritten (673 -> 521 lines) while adding four previously undocumented commands: - New `Start here` and `Authentication` sections. Setup is driven through the CLI: agents read `stash manifest --json` first, then trigger `stash auth login --json` and surface the verification URL for a human to approve, then run `stash init`. Authenticating before `init` matters -- init's auth step takes the interactive path and would otherwise try to open a browser on the agent's host. - New `Never read these` invariant, mirrored into the AGENTS.md doctrine: never read `~/.cipherstash/secretkey.json`, `~/.cipherstash/auth.json`, anything under `~/.cipherstash/workspaces/`, or `.env*`. The wizard already blocks these paths in code (`agent/interface.ts`), but the Claude Code, Codex and AGENTS.md targets had no written rule. - Documents `manifest`, `doctor`, `wizard` and `auth regions`, plus the non-interactive interface: per-command escape hatches (there is no global `--non-interactive`/`--yes`/`--json`), exit codes, the DATABASE_URL resolution order, and the `auth login --json` NDJSON event contract. - Corrects the db -> eql move. `db install`, `db upgrade` and `db status` are deprecated aliases that warn and forward; `db push`, `db activate`, `db validate`, `db test-connection` and `db migrate` remain in `db`. - Scopes `db push` / `db activate` as EQL v2 + CipherStash Proxy only, in both the skill and the README's recommended flow. - Adds the missing `--database-url`, `--eql-version`, `--prisma-next`, `--proxy`/`--no-proxy` and `--region` flags; corrects six programmatic API signatures; repoints the closed #447 citations at #585; and marks `stash env` as the non-functional stub it currently is. Also fixes the README's claim that `stash init` ends in a four-option agent handoff menu -- init is mechanical-only and chain-prompts to `stash plan`; the handoff belongs to `plan` / `impl`. Verified by diffing every command and flag in the skill against `stash manifest --json`, and by exercising the documented `auth login --json`, `auth regions --json`, `db status` deprecation and `env` stub behaviours.
🦋 Changeset detectedLatest commit: 9c673bb The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThis PR refreshes documentation for the Changesstash-cli documentation refresh
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/cli/src/commands/init/doctrine/AGENTS-doctrine.md`:
- Around line 18-21: The env-file restriction in AGENTS-doctrine.md is too broad
because it bans every .env* file while also requiring placeholders in
.env.example. Update the rule so it only forbids secret-bearing runtime env
files and explicitly allows .env.example for placeholder entries; keep the rest
of the secret-handling guidance unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01c9460d-36d1-4153-a534-267437710607
📒 Files selected for processing (4)
.changeset/stash-cli-skill-refresh.mdpackages/cli/README.mdpackages/cli/src/commands/init/doctrine/AGENTS-doctrine.mdskills/stash-cli/SKILL.md
There was a problem hiding this comment.
Pull request overview
This PR refreshes the shipped stash-cli agent skill and the published CLI README to match the current stash CLI surface, with emphasis on agent/non-interactive usage, authentication flow, and safety invariants around credentials.
Changes:
- Rewrites
skills/stash-cli/SKILL.mdto be manifest-driven (stash manifest --json/--help) and to document the current lifecycle (init → plan → impl → status) plus key commands/flags. - Updates the init doctrine (
AGENTS-doctrine.md) and the CLI README to reflect the new handoff boundaries and reinforce “don’t read secrets” guidance. - Adds a changeset to ship the refreshed bundled skill in the
stashnpm package.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| skills/stash-cli/SKILL.md | Major refresh of the bundled stash-cli skill to align with current CLI commands, flags, and agent workflow. |
| packages/cli/src/commands/init/doctrine/AGENTS-doctrine.md | Extends durable agent doctrine with explicit ~/.cipherstash credential-handling restrictions. |
| packages/cli/README.md | Corrects the quickstart narrative to place agent handoff in plan/impl (not init) and clarifies recommended flow. |
| .changeset/stash-cli-skill-refresh.md | Ships the skill/README refresh as a stash patch release. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses review on #594. **The read path after `encrypt cutover` is not automatic.** The skill claimed "Reads of `<col>` now decrypt transparently — no application read-path change." That is only true for CipherStash Proxy, which decrypts on the wire. For SDK users -- the majority -- `<col>` holds ciphertext after the rename swap, and read paths must go through the encryption client or they hand raw EQL payloads to end users. The CLI's own guidance already says so (`init/lib/setup-prompt.ts:296`): "Post-cutover, `<col>` holds ciphertext. Read code paths must decrypt before returning the value to callers." The claim was inherited from the old skill, and it had propagated to three places, all corrected here: - `encrypt cutover` now carries an explicit callout naming `decryptModel` / `encryptedSupabase` / `bulkDecryptModels`, with Proxy as the stated exception. - The `stash plan` state table described a cutover as "backfill + schema rename"; it is backfill, rename, read-path switch, drop. - "Why the split exists" said the cutover ends with a "rename-swap so reads decrypt", which begs the question. **`.env.example` is no longer caught by the secrets read-ban.** The doctrine forbade reading "any `.env*` file" in the same breath as instructing agents to add placeholder keys to `.env.example`. Narrowed to value-bearing env files, with `.env.example` called out as the exception. Mirrored in the skill. Verified: every command and flag still resolves against `stash manifest --json`; 406 unit tests pass.
|
Both review points were valid and are fixed in 1. Post-cutover read path (@copilot) — good catch, this was the serious oneThe skill claimed:
That's true only for CipherStash Proxy, which decrypts on the wire. For SDK users — the majority —
So the skill was contradicting the CLI's own doctrine, and this is a shipped artifact that lands in customer repos. It was inherited from the old skill rather than introduced here, and it had spread to three places, all now corrected:
2.
|
|
Agreed on leaving ✅ Action performedComments resolved. Approval is disabled; enable |
Completes the review fix on #594. The doctrine now says `.env.example` is exempt from the secrets read-ban; the wizard's runtime guard still blocked it. `SENSITIVE_FILE_PATTERNS` matched `/\.env($|\.)/`, which tests true against `.env.example`. Because `wizardCanUseTool` applies that guard to `Edit` and `Write` as well as `Read`, the agent could not create or edit the one file the doctrine explicitly instructs it to write. Committed env templates carry placeholder key names, not values. Narrow the rule with an anchored negative lookahead so `.env.example`, `.env.sample` and `.env.template` are reachable, while `.env`, `.env.local`, `.env.production` and near-miss names that do carry values (`.env.example.local`, `.env.example.bak`) stay blocked. `auth.json`, `secretkey.json` and credential files are untouched. Bash remains deliberately stricter: no env file, template or not, is reachable from a shell. `cat` isn't in the bash allowlist anyway, and Read/Write is the sanctioned path -- so there's no reason to widen the riskiest surface. Noted inline. Adds five cases covering the exemption, the near-miss names, and the Bash asymmetry. `pnpm --filter @cipherstash/wizard test` passes (150).
coderdan
changed the title
Why
skills/stash-cli/SKILL.mdhad drifted frompackages/cli(stash@0.17.1). The trigger was three changes it didn't reflect — thestash manifest --jsonregistry, the non-interactive/agent interface, and thedb→eqlmove — but the audit surfaced considerably more, including two correctness bugs (see Bug fixes below).The framing that matters: this skill is a shipped artifact, not internal documentation.
packages/cli/tsup.config.tscopies/skillsintodist/skills/, so it ships inside thestashnpm tarball.installSkills()copies it into the user's.claude/skills/or.codex/skills/at handoff time — for all three integrations.readBundledSkill()reads onlySKILL.md, strips the frontmatter, and inlines the body into the user'sAGENTS.mdfor Cursor/Windsurf/Cline.So a wrong command here becomes a wrong command in a customer's repo, and every line is context cost in their
AGENTS.md. The rewrite goes 673 → 525 lines while adding four commands and three sections, by teaching the agent to interrogate the CLI (stash manifest --json,<cmd> --help) for exact flags and reserving prose for what those can't convey: lifecycle, safety gates, and rationale.Bug fixes (found in review)
The post-cutover read path is not automatic. The skill said reads of
<col>afterencrypt cutover"decrypt transparently — no application read-path change." That is true only for CipherStash Proxy, which decrypts on the wire. For SDK users — the majority —<col>holds ciphertext after the rename swap, and unwired read paths return raweql_v2_encryptedpayloads to end users. The CLI already says exactly this inpackages/cli/src/commands/init/lib/setup-prompt.ts:296. The claim was inherited from the old skill and had spread to three places, all corrected. Thanks @copilot.The wizard's agent guard blocked
.env.example.SENSITIVE_FILE_PATTERNSinpackages/wizard/src/agent/interface.tswas/\.env($|\.)/, which teststrueagainst.env.example. SincewizardCanUseToolapplies that guard toEditandWriteas well asRead, the wizard's agent could not create or edit the one file the doctrine explicitly instructs it to write. Now an anchored negative lookahead:/\.env($|\.(?!(example|sample|template)$))/.env.example/.env.sample/.env.templateare reachable..env,.env.local,.env.production, and near-miss names that do carry values (.env.example.local,.env.example.bak) stay blocked, as doauth.json,secretkey.jsonand credential files. Bash remains deliberately stricter — no env file is reachable from a shell;catisn't in the bash allowlist anyway and Read/Write is the sanctioned path, so there was no reason to widen the riskiest surface. Five new test cases. Thanks @coderabbitai and @copilot.What else is in it
New
Start here+Authentication. All setup is driven through the CLI. Agents readstash manifest --jsonfirst, then triggerstash auth login --jsonand surface the verification URL for a human to approve, then runnpx stash init(--supabaseon Supabase).Authenticating before
initis load-bearing:init's authenticate step callslogin()with no options (init/steps/authenticate.ts:59-61), so it takes the interactive path and would try to open a browser on the agent's host, and fails in non-TTY without--region.New
Never read theseinvariant, mirrored intoAGENTS-doctrine.md: never read~/.cipherstash/secretkey.json,~/.cipherstash/auth.json, anything under~/.cipherstash/workspaces/, or a value-bearing env file..env.exampleis the stated exception.Corrects the
db→eqlmove, which was half-right.db install,db upgradeanddb statusare deprecated aliases that warn and forward.db push,db activate,db validate,db test-connectionanddb migrategenuinely remain indb.Documents four commands the skill omitted entirely:
manifest,doctor,wizard,auth regions. Plus the non-interactive interface — there is no global--non-interactive/--yes/--json; it'sstdin.isTTY && CI ∉ {1,true}with a per-command escape hatch each.Scopes
db push/db activateas EQL v2 + CipherStash Proxy only (true today), in both the skill and the README flow. Structured so that when #586 lands and flips the v3 default, only the version line needs editing.Other corrections: adds the missing
--database-url,--eql-version,--prisma-next,--proxy/--no-proxy,--regionflags; corrects six programmatic API signatures (loadStashConfig,loadBundledEqlSql,downloadEqlSql,PermissionCheckResult.isSuperuser, theeqlVersionoption onEQLInstaller, the undocumentedresolveDatabaseUrlexport); repoints the three citations of closed #447 at open #585; and marksstash envas the non-functional stub it is (double-gated, andfetchProdCredentials()returnsundefinedunconditionally).README fix: it claimed
stash initends in a four-option agent-handoff menu. It doesn't — init is mechanical-only and chain-prompts tostash plan; the handoff belongs toplan/impl. This README is published to npm.Verification
Diffed against the built CLI rather than proofread.
stash manifest --json. The only two that don't (--non-interactive,--yes) appear solely in the sentence stating they don't exist.stash eql install --helpmatches the skill's flag table line for line.CI=true stash auth login --jsonemits exactly the documented{"status":"error","code":"region_required",...}and exits 1.stash auth regions --jsonreturns the documented{slug,label}array.stash db statusprints the deprecation warning and forwards toeql status.stash envwarns it's experimental and does nothing.dist/skills/, and the frontmatter stripper yields a body starting at# CipherStash CLIwith no stray---.pnpm --filter stash test— 406 passing.pnpm --filter @cipherstash/wizard test— 150 passing.The
~/.cipherstashprofile layout was confirmed by listing filenames only, and the NDJSON event ordering by readinglogin.ts— no file under~/.cipherstash/was opened, andauth loginwas not re-run against the live device flow.Note
Found but deliberately not fixed here:
packages/cli/README.md:6advertises "encrypted secrets management" and thestash-secretsskill description says the same, butstash secretsreturnsUnknown command. Handled in #595.