feat(session): per-command scope gating with console-first remediation - #687
Open
jennijuju wants to merge 8 commits into
Open
feat(session): per-command scope gating with console-first remediation#687jennijuju wants to merge 8 commits into
jennijuju wants to merge 8 commits into
Conversation
Session-key mode required all four FWSS permissions on every command, so a least-privilege key (e.g. upload-only) could not run the CLI at all. Commands now declare the permissions they need; the preflight checks only those and passes the same set to Synapse.create (whose own gate otherwise defaults to all four). Per-command sets: add/import = CreateDataSet+AddPieces; rm = SchedulePieceRemovals; data-set terminate = TerminateService; pinning server = CreateDataSet+AddPieces+SchedulePieceRemovals; reads = none. A missing scope now fails with a console-first remediation: the target network, then a Filecoin Pay console deep link (?authorize=<addr>&scopes=<missing>) to approve with the owner wallet, then the owner-only CLI commands (session authorize/create --scopes). Distinguishes 'not authorized at all' (wrong network / never granted / revoked, with a --network hint) from 'missing this scope'. Console URL resolves via CONSOLE_URL env or a mainnet default (TEMPORARY helper to dedupe with the console-pairing branch).
Both networks resolve to the production console so gating errors carry a working ?authorize=&scopes= deep link out of the box; CONSOLE_URL and the explicit override still win for local/preview consoles. Must not ship before the console's session-keys page deploys — opened as a pair with the console PR.
A calibration remediation link approved by a mainnet-connected wallet silently granted the scopes on mainnet (hit in testing). The console refuses to prefill on a chain mismatch when the link names its network.
No production caller ever passed it (only resolveConsoleUrl(chainId)), and CONSOLE_URL already covers the override use case.
…etail - buildAuthorizeUrl lowercases the address: the console validates with viem strict isAddress, which silently rejects wrong-checksum mixed-case; lowercase always passes - preflight failure restores the per-scope 'expired at <ts>' vs 'never granted' detail the pre-gating wording carried - README: replace FWSS jargon in the permissions intro; scope table uses canonical camelCase ids matching --scopes on-the-wire casing - tests: expired-scope regression; scope-id derivation lockstep cross-check against PermissionNames
The deployed console serves calibration under a path prefix, not the bare host — bare-host remediation links 404 on calibration. Also trims the helper's comments: the stale TEMPORARY/dedupe note is gone (this file is the canonical console-URL builder; the pairing/login work builds on it) and the merge-order NOTE moves to the PR description.
Review found the preflight comment unreadable; rewritten to say when the check runs, what happens on a missing grant, and that read-only commands leave it unset.
FilOzzy
added
team/filecoin-pin
jennijuju
requested review from
SgtPooki and
beck-8
and removed request for
SgtPooki
August 28, 2026 03:04
Contributor
There was a problem hiding this comment.
Pull request overview
Adds least-privilege session-key permission gating and console-first authorization remediation.
Changes:
- Declares required scopes per mutating command.
- Adds scoped preflight checks and authorization deep links.
- Documents and tests scope behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
README.md |
Documents command scope requirements. |
src/add/add.ts |
Requires upload scopes. |
src/core/session/console-url.ts |
Builds console authorization links. |
src/core/synapse/index.ts |
Implements permission preflight and remediation. |
src/data-set/run.ts |
Requires termination scope. |
src/filecoin-pinning-server.ts |
Declares server scopes. |
src/import/import.ts |
Requires upload scopes. |
src/rm/remove-all-pieces.ts |
Requires removal scope. |
src/rm/remove-piece.ts |
Requires removal scope. |
src/test/mocks/synapse-core-session-key.ts |
Adds permission-name fixtures. |
src/test/unit/console-url.test.ts |
Tests console URLs. |
src/test/unit/session-scopes.test.ts |
Verifies scope-name derivation. |
src/test/unit/synapse-service.test.ts |
Tests permission preflight behavior. |
src/utils/cli-auth.ts |
Forwards command permissions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Match the SDK's own permission gate to this command's needs; without it | ||
| // Synapse.create defaults to requiring all FWSS permissions and re-rejects a | ||
| // subset key that our preflight already accepted. | ||
| synapseOptions.requiredPermissions = config.requiredPermissions ?? [] |
| footnotes.join('\n') | ||
| ) | ||
| const consoleUrl = resolveConsoleUrl(chainId) | ||
| const lines = neverAuthorized ? [problem, ''] : [problem, ...scopeDetails, ''] |
| requiredPermissions: [AddPiecesPermission, CreateDataSetPermission], | ||
| } | ||
|
|
||
| await expect(initializeSynapse(config, logger)).resolves.toBeDefined() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
e2e preview:https://inbrowser.link/ipfs/bafybeicfitpkj5i5nu37o2dlntbibzbzsl6dul3mvoh2fy3dn34qguxv64
depend on FilOzone/filecoin-pay-explorer#355
part 1 of our session key move #682
currently session key mode preflights all four FWSS permissions on every command — even pure reads like
payments status— so a least-privilege key (upload-only) can't use the CLI at all. Since SessionKeyRegistry supports per-permission grants, the chain already allows subsets; only the CLI refused them. (inherited from synapse)Commands now declare what they need (checked in the preflight AND passed to
Synapse.create, whose own gate otherwise defaults to all four):payments status,data-set ls,provider ls, …)add/importrmdata-set terminateA missing scope fails up front with a console-first message: the problem, with per-scope detail (
expired at <timestamp>vsnever granted, read from the on-chain expirations) and the network it happened on;a Filecoin Pay console deep link (
…/console/session-keys?authorize=<addr>&scopes=<missing>&network=<net>) to approve with the owner wallet; then the owner-only CLI alternatives (session authorize/create --scopes, from the base PR). A delegate is never told to run a root-key command as their own action.