feat(cli): add read-only profile flag to block write commands#1147
Conversation
Profiles created with `--readonly` (or toggled via `profile update --readonly`) refuse every mutating command — apply, remove, workspace/secret/vault/folder/PAT mutations, workflow start/resume, executor trigger, staticwebsite/erd deploy, function test-run, and authconnection authorize/revoke. The guard fires before any prompt or RPC, so editor users can point `TAILOR_PLATFORM_PROFILE` at a viewer-style profile and trip a clear `PROFILE_READONLY` error instead of an accidental apply. Coverage stays honest with a fail-closed test: every runnable command that is not on the read-only/local allowlist must call `assertWritable`, and the matcher anchors on `await assertWritable(` so dropping the call without dropping the import still trips CI. Refs platform-planning#1109.
🦋 Changeset detectedLatest commit: 9a0f5a4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
⚡ pkg.pr.new@tailor-platform/sdk@tailor-platform/create-sdk
|
This comment has been minimized.
This comment has been minimized.
The fail-closed allowlist exempted the parent `api` command and missed the top-level `query` command entirely. Both can mutate platform state: `tailor-sdk api <endpoint>` invokes arbitrary OperatorService methods including Create/Update/Delete; `tailor-sdk query` forwards SQL and GraphQL through executeScript, which can run DELETE/UPDATE or GraphQL mutations. Guard both with assertWritable and extend the coverage test to walk command files outside src/cli/commands/. Also let `profile update --readonly` / `--no-readonly` skip remote user and workspace validation when neither user nor workspace is changing, so the readonly flag can be cleared offline or with an expired token.
`assertWritable` used `??` to fall through to TAILOR_PLATFORM_PROFILE, but `loadAccessToken` / `loadWorkspaceId` use `||`. With `--profile ""` the guard saw an empty explicit profile and returned silently while the loaders still picked up a readonly profile from the env var, so mutating commands could run anyway. Use truthy fallback in the guard to match the loaders.
Polish pass: drop em-dashes from comments and changeset, collapse the two consecutive early returns in assertWritable into one, and reword the rhetorical-question test name added in the previous commit.
Cover the key new guarantees: --readonly / --no-readonly skips remote user / workspace validation, --no-readonly clears the field on disk, and mixing --user with --readonly still validates remote.
This comment has been minimized.
This comment has been minimized.
📖 Docs Consistency Check✅ No inconsistencies found between documentation and implementation. Checked areas:
Notes:
|
| name: profileName, | ||
| user: profileUser, | ||
| workspaceId: workspace.id, | ||
| readonly: false, |
There was a problem hiding this comment.
So your judgment is that there is no need to make the profile read-only at the time the workspace is created, correct?
| // SQL/GraphQL submitted here is forwarded as-is; we cannot reliably | ||
| // classify a statement as read-only, so block the whole command under a | ||
| // readonly profile. | ||
| await assertWritable({ profile: args.profile }); |
There was a problem hiding this comment.
Regarding the manipulation of application data, permission management is expected to be handled by machineUser, so I don't think it's necessary to call assertWritable here.
There was a problem hiding this comment.
The same applies to workflow execution and test runs.
It might be worth considering making it possible to set a default machine user in the profile separately.
| // Direct API calls can target any OperatorService method, including | ||
| // Create/Update/Delete. Block all of them under a readonly profile rather | ||
| // than try to classify endpoints by name. |
Adds `--readonly` to `workspace create` so the profile written via `--profile-name` can opt into read-only mode at creation time, matching the option already exposed by `profile create`. Passing `--readonly` without `--profile-name` warns and continues so the workspace is still created. Also documents that the existing `assertWritable()` call in `workspace create` resolves the active profile from `TAILOR_PLATFORM_PROFILE` only (the command does not accept `--profile`).
Application-data operations run through a configured machine user whose own permissions already govern what the workflow / function / SQL or GraphQL request can do, so gating them on the operator's local profile flag duplicates a concern that lives on the platform side. Drop `assertWritable` from `query`, `workflow start`, `workflow resume`, and `function test-run`, and move those files (plus the previously omitted `query/index.ts` external runnable path) into the read-or-local allowlist so the fail-closed coverage test stays meaningful. `executor trigger` stays guarded: it calls the production `TriggerExecutor` RPC with the operator token and records a job on the workspace, so it is a platform-state mutation regardless of the machine user that ultimately executes the job. Narrow the `PROFILE_READONLY` details text accordingly so it stops implying that every write is blocked.
646e9d6 to
37f6ec7
Compare
Code Metrics Report (packages/sdk)
Details | | main (ae7aa27) | #1147 (dec95a3) | +/- |
|--------------------|----------------|-----------------|-------|
+ | Coverage | 60.9% | 61.3% | +0.3% |
| Files | 358 | 359 | +1 |
| Lines | 12217 | 12256 | +39 |
+ | Covered | 7448 | 7514 | +66 |
+ | Code to Test Ratio | 1:0.4 | 1:0.4 | +0.0 |
| Code | 80092 | 80660 | +568 |
+ | Test | 32936 | 33404 | +468 |Code coverage of files in pull request scope (22.2% → 27.6%)DetailsSDK Configure Bundle Size
Runtime Performance
Type Performance (instantiations)
Reported by octocov |
| "@tailor-platform/sdk": minor | ||
| --- | ||
|
|
||
| Add `--readonly` flag to `profile create`, `profile update`, and `workspace create` (when `--profile-name` is given) so editor users can use a viewer-style profile by default. Read-only profiles block platform-state mutations driven by the operator's bearer token (`apply`, `remove`, `workspace create/delete/restore`, `secret create/update/delete`, `tailordb migrate set`, `tailordb truncate`, `tailordb erd deploy`, `executor trigger`, `staticwebsite deploy`, `authconnection authorize/revoke`, organization / folder / PAT / workspace-user mutations, and direct `api <endpoint>` calls) with a `PROFILE_READONLY` error. Application-data operations executed under a machine user (`query`, `workflow start/resume`, `function test-run`) are not gated because the machine user's own permissions already govern those mutations. Switch profile or run `profile update <name> --no-readonly` to lift the restriction. Profile management itself stays available so the flag can always be cleared. `profile update` skips remote user / workspace validation when only `--readonly` / `--no-readonly` is changing, so the flag can be cleared offline or with an expired token. |
There was a problem hiding this comment.
I just noticed that it feels a bit strange that profile update results in an error and you have to use profile update --no-readonly.
How about adding a "writable" flag that is mutually exclusive with "readonly"?
There was a problem hiding this comment.
I'm just working on a feature for renaming negation flags.
toiroakr/politty#393
There was a problem hiding this comment.
tailor-sdk profile update --permission write/read ?
There was a problem hiding this comment.
Switched to --permission <write|read> on profile create, profile update, and workspace create in 9a0f5a4. The persisted readonly: true field stays so existing v2 configs round-trip unchanged; only the CLI surface (and the ProfileInfo JSON output, now permission) moves to the new vocabulary.
Per review feedback (#1147), the boolean negation pair `--readonly` / `--no-readonly` is awkward to use in `profile update`. Switch the user-facing API on `profile create`, `profile update`, and `workspace create` to an explicit `--permission <write|read>` enum so both directions are positive and self-describing. The persisted YAML field stays `readonly: true` so existing v2 configs round-trip unchanged; only the CLI surface and the `ProfileInfo` JSON output (now `permission: "write" | "read"`) move to the new vocabulary.
Adds a per-profile
readonlyflag so editor-permission users can default to a viewer-style workflow without dropping write capability project-wide. While a read-only profile is in scope, mutating CLI commands that operate on platform state via the operator's bearer token (apply,remove,workspace/secret/vault/folder/PATmutations,executor trigger,staticwebsite/erd deploy,authconnection authorize/revoke, organization / workspace-user mutations, and directapi <endpoint>calls) fail fast with aPROFILE_READONLYerror before any prompt or RPC.Application-data operations that run under a machine user (
query,workflow start/resume,function test-run) are not gated by this flag because the machine user's own permissions already govern those mutations.Profile management itself stays open so the flag can always be cleared with
tailor-sdk profile update <name> --no-readonly.Usage
Main Changes
readonly?: booleanto the persisted profile schema; existing v2 configs round-trip unchanged.assertWritable({ profile })helper resolves the active profile from--profileorTAILOR_PLATFORM_PROFILEand throws aCLIError { code: "PROFILE_READONLY" }when the profile is read-only.run()callsawait assertWritable(...)at the top, before confirmations or token refreshes.profile create/profile updateaccept--readonly(and--no-readonlyto clear);workspace createaccepts--readonlywhen used with--profile-nameto bootstrap a read-only profile alongside the new workspace;profile listand theProfileInfoJSON output expose the flag.cli/commands/and requires anything not on a small read-only/local allowlist (which now includes the machine-user-driven app paths) to callawait assertWritable(.Notes
TAILOR_PLATFORM_TOKENdirect injection and--workspace-idwithout a profile are out-of-band paths whose authorization is governed by the bearer token itself; documented in theassertWritableJSDoc and the changeset.