Skip to content

feat(auth): auth-mode seam with injectable signer - #458

Open
SgtPooki wants to merge 13 commits into
masterfrom
spike/ows-wallet
Open

feat(auth): auth-mode seam with injectable signer#458
SgtPooki wants to merge 13 commits into
masterfrom
spike/ows-wallet

Conversation

@SgtPooki

@SgtPooki SgtPooki commented May 6, 2026

Copy link
Copy Markdown
Collaborator

What changed

Adds the auth-mode seam the wallet-auth epic (#457) builds on: source-aware precedence resolution for the mutually exclusive auth flags (an explicit flag beats an env var; two explicit modes are a hard error), and an injectable viem Account accepted by initializeSynapse() so signer backends plug in without SDK changes.

This PR began as an OpenWallet Standard spike. The OWS backend was removed in 7928f6f after an upstream health check: no maintainer activity since 2026-06-19, the sole human maintainer removed from npm, and an unanswered policy-bypass report (open-wallet-standard/core#228). Evidence: OWS proof-of-life memo.

How to verify

pnpm install && pnpm run build && pnpm exec vitest run. Precedence behavior is covered in src/test/unit/cli-auth.test.ts (explicit-beats-env, lone session-key halves, conflict errors, devnet fallback).

Notes

No user-facing flags added or removed relative to master; the --wallet/OWS options that existed only on this branch are gone. First consumers of the seam: keystore (#681) and session keys in CI (#682).

Wires @open-wallet-standard/adapters/viem into the existing AccountConfig
path so signing can be delegated to OWS (private key never leaves the OWS
core).

Adds:
- --wallet <id> / OWS_WALLET_ID
- --wallet-passphrase / OWS_WALLET_PASSPHRASE

Resolver order: --wallet (OWS) takes precedence over --private-key /
PRIVATE_KEY. devnet auto-resolve still applies when no auth supplied.

Refs #457
@FilOzzy FilOzzy added team/filecoin-pin "Filecoin Pin" project is a stakeholder for this work. team/fs-wg FOC working group is a stakeholder for this work, and thus wants to track it on their project board. labels May 6, 2026
@FilOzzy FilOzzy added this to FOC May 6, 2026
@github-project-automation github-project-automation Bot moved this to 📌 Triage in FOC May 6, 2026
@SgtPooki SgtPooki self-assigned this May 6, 2026
…port

@open-wallet-standard/core ships napi-rs prebuilt binaries only for
linux-x64-gnu, linux-arm64-gnu, darwin-x64, and darwin-arm64. A static
import of @open-wallet-standard/adapters/viem triggers a require of the
native binding at module evaluation time, crashing CLI startup on Windows
runners (and any platform without a published prebuilt) even when the
user never asks for OWS auth.

Load the adapter via ESM dynamic `import()` inside getOwsAccount() so the
native binding only resolves when --wallet / OWS_WALLET_ID is actually
used. Surface a helpful error if the platform lacks a prebuilt.

Makes getOwsAccount() and parseCLIAuth() async; updates the 12 call sites
(already inside async action handlers) to await.

Refs #457
@SgtPooki
SgtPooki force-pushed the spike/ows-wallet branch from 63c6524 to c4e35e2 Compare May 6, 2026 20:00
OWS wallets typically expose both an eip155:* (EVM) account and a
fil:* (native Filecoin) account derived from the same seed. filecoin-pin
signs via FEVM through synapse-sdk and only uses the eip155 entry; the
fil:* / f1 address is not used. Users seeing both in `ows wallet list`
could reasonably assume the native one is the right pick.

Add a defensive check that rejects any non-EVM address returned by the
adapter, with an error message pointing at `ows wallet list`. Also
document the FEVM-only scope in the module doc comment.

Refs #457
@BigLep BigLep moved this from 📌 Triage to ⌨️ In Progress in FOC May 7, 2026
@BigLep BigLep moved this from ⌨️ In Progress to 🐱 Todo in FOC May 12, 2026
@BigLep BigLep added this to the M4.5: GA Fast Follows milestone May 25, 2026
@SgtPooki SgtPooki changed the title feat(auth): spike OpenWallet Standard wallet support feat(auth): add OpenWallet Standard wallet support Jun 29, 2026
@SgtPooki
SgtPooki marked this pull request as ready for review June 29, 2026 22:16
@SgtPooki
SgtPooki requested review from BravoNatalie and Copilot June 29, 2026 22:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds OpenWallet Standard (OWS) wallet authentication to filecoin-pin’s CLI auth resolver, allowing users to supply an OWS wallet ID (and optional passphrase) so signing can be performed by an OWS-managed viem Account instead of exposing raw private keys.

Changes:

  • Added --wallet/OWS_WALLET_ID and --wallet-passphrase/OWS_WALLET_PASSPHRASE CLI options and wired them into the shared auth path.
  • Introduced src/core/ows/index.ts to lazily load the OWS viem adapter and produce a viem Account.
  • Updated CLI flows to await parseCLIAuth() after making it async to support OWS account resolution.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/utils/cli-options.ts Adds new Commander options for OWS wallet ID + passphrase.
src/utils/cli-auth.ts Makes auth parsing async; adds OWS account resolution and precedence.
src/core/ows/index.ts New OWS integration module that builds a viem Account via dynamic import.
src/core/synapse/index.ts Updates missing-auth error message to mention OWS wallet auth.
src/add/add.ts Awaits async auth parsing before Synapse init.
src/import/import.ts Awaits async auth parsing before Synapse init.
src/payments/auto.ts Awaits async auth parsing before Synapse init.
src/payments/deposit.ts Awaits async auth parsing before Synapse init.
src/payments/fund.ts Awaits async auth parsing before Synapse init.
src/payments/status.ts Awaits async auth parsing before Synapse init.
src/payments/withdraw.ts Awaits async auth parsing before Synapse init.
src/rm/remove-all-pieces.ts Awaits async auth parsing before Synapse init.
src/rm/remove-piece.ts Awaits async auth parsing before Synapse init.
package.json Adds OWS core + adapter dependencies.
pnpm-lock.yaml Locks OWS dependencies and platform-specific optional packages.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/utils/cli-auth.ts Outdated
@SgtPooki

Copy link
Copy Markdown
Collaborator Author

@HavenCTO @Apha205, updated the PR.. feel free to leave feedback

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 4 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Comment thread src/utils/cli-auth.ts Outdated
Comment thread src/utils/cli-auth.ts Outdated
Comment thread src/utils/cli-options.ts Outdated
Comment thread src/utils/cli-options.ts Outdated
Comment thread src/utils/cli-auth.ts Outdated
Comment thread src/utils/cli-auth.ts
* @returns Synapse setup config (validation happens in initializeSynapse)
*/
export function parseCLIAuth(options: CLIAuthOptions): SynapseSetupConfig {
export async function parseCLIAuth(options: CLIAuthOptions): Promise<SynapseSetupConfig> {

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.

continuing on my precedence suggestion below, the complexity in here now deserves a bunch of tests to show it does what you expect, and document what you expect too

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

preferences should be much more clear now

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.

no more need for async in here, you can drop that and all of the await additions to make the diff shrink significantly

@rvagg

rvagg commented Jul 23, 2026

Copy link
Copy Markdown
Member

@SgtPooki did your testing extend beyond the payments command into actual data onboarding? This JSON.stringify looks like a problem for our bigints in 712 payloads, does it get past this? https://github.com/open-wallet-standard/core/blob/76c8c67d6c73bf57805432fc15a2e48d12168c6c/bindings/node-adapters/src/viem.js#L36

@SgtPooki

Copy link
Copy Markdown
Collaborator Author

@SgtPooki did your testing extend beyond the payments command into actual data onboarding? This JSON.stringify looks like a problem for our bigints in 712 payloads, does it get past this? open-wallet-standard/core@76c8c67/bindings/node-adapters/src/viem.js#L36

I didn't do super thorough testing, but I have now. I fixed this in our implementation here, added unit tests, and also opened an upstream fix: github.com/open-wallet-standard/core#242

SgtPooki added 4 commits July 27, 2026 14:11
The OWS viem adapter serializes typed data with JSON.stringify, which
throws on the bigints every synapse-sdk EIP-712 payload carries
(clientDataSetId, nonce, pieceIndex, permit value/deadline). Override
signTypedData to serialize the message the way the OWS core accepts:
bigints as even-length hex, with EIP712Domain injected when absent.

signMessage and signTransaction are left to the adapter.

Validated byte-identical to viem's privateKeyToAccount, and confirmed
on-chain (AddPieces, SchedulePieceRemovals) on Calibration.
Auth options merged from CLI flags and env vars could let an env var
silently beat an explicit flag. Capture each flag's source (cli vs env)
in a preAction hook and resolve precedence in parseCLIAuth: an explicit
flag wins over env, and two modes from the same tier is a hard error.

Session-key mode competes only when both halves are present, so a lone
--wallet-address or --session-key no longer outranks a complete mode.
Interactive setup prompted for a private key whenever one was not passed,
then handed it to parseCLIAuth. With an OWS wallet supplied that tripped
the mutual-exclusion check. Skip the prompt only when a sign-capable mode
is present: an OWS wallet or a complete session key. A view address and a
lone session-key half cannot run setup, so they still prompt.
Bumps @open-wallet-standard/core and /adapters from 1.3.2 to 1.4.2,
the release that adds trusted publishing.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (3)

src/utils/cli-auth.ts:187

  • OWS mode precedence is determined only from the wallet option source (const owsSource = sourceOf('wallet', ...)). If OWS_WALLET_ID is set via env but the user passes --wallet-passphrase explicitly to disambiguate from an env PRIVATE_KEY, parseCLIAuth() will still treat both modes as env-sourced and throw a conflict. This also contradicts the comment that a mode spanning several options should take the strongest source (like the session-key mode does). Consider tracking walletPassphrase in AuthOptionSources and using strongest(walletSource, walletPassphraseSource) when computing the OWS candidate source.
  const walletAddressSource = sourceOf('walletAddress', walletAddress)
  const sessionKeySource = sourceOf('sessionKey', sessionKey)
  const sessionSource =
    walletAddressSource && sessionKeySource ? strongest(walletAddressSource, sessionKeySource) : undefined
  if (sessionSource)
    candidates.push({ mode: 'sessionKey', source: sessionSource, label: '--wallet-address/--session-key' })
  const owsSource = sourceOf('wallet', owsWalletId)
  if (owsSource) candidates.push({ mode: 'ows', source: owsSource, label: '--wallet/OWS_WALLET_ID' })

src/utils/cli-options.ts:20

  • collectAuthOptionSources() tracks provenance for mutually exclusive auth options, but AUTH_OPTION_NAMES currently omits walletPassphrase. If a user relies on env OWS_WALLET_ID and provides --wallet-passphrase explicitly (or vice versa), parseCLIAuth can't treat the OWS mode as explicitly selected, which can cause avoidable "Conflicting authentication options" errors when other env auth (e.g. PRIVATE_KEY) is also set. Include walletPassphrase in the collected option names (and in AuthOptionSources) so OWS mode can apply the same "strongest source wins" rule as session-key mode.
 * Commander attribute names for the mutually exclusive auth flags whose
 * provenance {@link parseCLIAuth} needs to resolve precedence.
 */
const AUTH_OPTION_NAMES = ['privateKey', 'wallet', 'walletAddress', 'sessionKey', 'viewAddress'] as const

src/utils/cli-auth.ts:123

  • The conflict error text says "Provide exactly one signing mode", but the candidate set includes read-only mode (--view-address), which is explicitly non-signing. This can be confusing when users hit a conflict involving view-only auth. Consider wording this as "authentication mode" (or similar) instead.
  const conflict = (modes: AuthModeCandidate[], envOnly: boolean): Error => {
    const labels = modes.map((c) => c.label).join(' and ')
    const hint = envOnly ? ' Pass an explicit flag to disambiguate.' : ''
    return new Error(`Conflicting authentication options: ${labels}. Provide exactly one signing mode.${hint}`)
  }

@SgtPooki
SgtPooki requested review from Copilot and rvagg and removed request for Copilot July 28, 2026 17:18
Comment thread src/core/ows/index.ts Outdated
* fields; the hex form here is unsigned.
*/
function bigintToOwsHex(value: bigint): string {
if (value < 0n) return value.toString()

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.

what's going on here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

just to prevent 0x-1 when trying to convert from any negative bigint passed through OWS. shouldn't happen based on looking through code in syanpse-sdk, but it could happen in the future..

@rvagg rvagg 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.

tbh this was a very light re-review; the only thing that stands out to my eye is the hex <0n thing, otherwise as long as this works in practice then it seems good to me to ship as an experimental feature
@jennijuju might like to try and integrate this into the skill(s) she's working on

@github-project-automation github-project-automation Bot moved this from 🔎 Awaiting review to ✔️ Approved by reviewer in FOC Aug 3, 2026
@Apha205

Apha205 commented Aug 4, 2026

Copy link
Copy Markdown

@SgtPooki @rvagg , tested against Haven CLI

The CLI can authenticate and sign transactions using an OWS-backed viem Account (OWS_WALLET_ID) without requiring access to a raw private key.

  • OWS_WALLET_ID successfully resolved to EVM account address
  • Synapse initialized using the OWS-backed viem Account path and selected two Calibration storage providers
  • pieces_added transactions were submitted and signed by the OWS-managed wallet
  • Upload completed successfully

No PRIVATE_KEY was set or used during the test.

Screenshots from the debug logs attached below.
Screenshot 2026-08-04 045802

Screenshot 2026-08-04 051138 Screenshot 2026-08-04 045614

@SgtPooki

SgtPooki commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

it seems good to me to ship as an experimental feature

@rvagg do you have any preference on how we communicate this as an "experimental feature" ?

@rvagg

rvagg commented Aug 6, 2026

Copy link
Copy Markdown
Member

@SgtPooki nope; I'll leave that to you, I'm not demanding you even take action on making it "experimental", I'm just noting that "OpenWallet" is pretty young and not very well used and obviously has rough edges; it suggests some caution in enshrine it in a way that might be easy to yank it out in the future.

Drop @open-wallet-standard/* and the ows auth mode. Upstream is
unmaintained (no maintainer activity since 2026-06-19) with an
unanswered policy-bypass report (open-wallet-standard/core#228), so an
experimental flag would ship risk without benefit over the planned
encrypted keystore. The spike's auth seam survives: source-aware
precedence resolution and the pre-created viem Account path in
initializeSynapse. See #457.
@SgtPooki

Copy link
Copy Markdown
Collaborator Author

Following up on the "pretty young and not very well used" concern: I dug into OWS upstream health and you were more right than either of us knew. No maintainer commit, review, or merge since 2026-06-19; the sole human maintainer was removed from the npm packages on 2026-08-03 and his GitHub account is gone; SECURITY.md routes to that same removed identity; and a policy-bypass report (#228, wallet delete --confirm skips the policy engine and all safety gates) has sat unanswered since May. Full evidence with dates: https://gist.github.com/SgtPooki/c44b6a8a667bb2c80a48d5e0767b8cce

So rather than enshrine it, or even flag it experimental, I removed the OWS backend from this PR entirely (7928f6f). What remains is the part worth merging: source-aware auth precedence resolution and the injectable viem Account seam in initializeSynapse, which the keystore and session-key work in #457 builds on. I'll update the PR title/description to match.

@rvagg rvagg 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.

approval pending removal of async and the awaits and renaming this before it lands as a misnamed commit

@SgtPooki SgtPooki changed the title feat(auth): add OpenWallet Standard wallet support feat(auth): auth-mode seam with injectable signer Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team/filecoin-pin "Filecoin Pin" project is a stakeholder for this work. team/fs-wg FOC working group is a stakeholder for this work, and thus wants to track it on their project board.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants