fix: enforce ensureCloudToken invite policy

[jchris]

Summary

• block uninvited users from reusing an appId binding and show guidance to request an invite
• keep auto-redeem flow working by rechecking bindings after ensureUser redeem logic
• restore regression test that verifies the new policy and message

Testing

• npm test -- dashboard/backend/tests/db-api.test.ts

Summary by CodeRabbit

Release Notes

• New Features

  • Enhanced cloud token generation with improved parameter handling

• Bug Fixes

  • Added stronger validation preventing unauthorized users from accessing appIds they weren't invited to
  • Improved detection of duplicate binding conflicts

• Tests

  • Added test coverage for unauthorized appId access scenarios

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Avoid filtering existing appId binding by requested tenant

The new getExistingAppIdBinding reuses the caller-provided filters (tenant/ledger) when checking whether an appId is already registered. If an uninvited user supplies a different tenant (or ledger) than the one that owns the appId, this lookup returns null and ensureCloudToken proceeds to create a second binding as long as maxAppIdBindings is not exceeded. That bypasses the intended “appId already registered, ask the owner for an invite” guard. The existing-binding check should ignore tenant/ledger filters and search by appId/env only so uninvited users cannot opt out of the policy by passing alternate scopes.

Useful? React with 👍 / 👎.

[coderabbitai]

Walkthrough

This pull request updates the ensureCloudToken function to accept an optional ictx parameter and implements a memoized single-flight mechanism for ensureUser calls. It introduces logic to detect and reject requests when an existing app ID binding is found for the requesting user, with a new test case validating this unauthorized access scenario.

Changes

Cohort / File(s)	Summary
Cloud token function update dashboard/backend/public/ensure-cloud-token.ts	Updated function signature to accept optional ictx parameter; introduced ensureUserOnce memoization to cache user resolution results; added getExistingAppIdBinding helper to check for conflicting bindings; refactored control flow to first check existing bindings, then resolve user info once, and verify membership and admin status.
Cloud token authorization test dashboard/backend/tests/db-api.test.ts	Added new test case validating that unauthorized users without invites are rejected when attempting to access an existing app ID binding, with error message verification.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Function signature and memoization logic: Verify the ensureUserOnce memoization correctly caches results within a single invocation and is thread-safe
• Binding conflict detection: Ensure getExistingAppIdBinding properly identifies existing bindings and the error early-exit path is correct
• Control flow changes: Confirm the reordered steps (check existing binding → memoize user resolution → verify membership) don't introduce regressions or edge cases

Possibly related PRs

• feat: Expose ensureCloudToken via HTTP API #1440: Adds HTTP routing and type-guard exposure for the ensureCloudToken function that is being modified in this PR.

Suggested reviewers

• mabels

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main change: enforcing an invite policy in ensureCloudToken to block uninvited users from reusing appId bindings.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jchris/appid-invite-policy

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0c5f5be and 408f218.

📒 Files selected for processing (2)

• dashboard/backend/public/ensure-cloud-token.ts (4 hunks)
• dashboard/backend/tests/db-api.test.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-09-10T09:53:26.979Z

Learnt from: mabels
Repo: fireproof-storage/fireproof PR: 1130
File: use-fireproof/react/use-attach.ts:186-199
Timestamp: 2025-09-10T09:53:26.979Z
Learning: In use-fireproof/react/use-attach.ts, the current attach flow calls database.attach immediately via KeyedResolvOnce, which can run before a token is available and won't re-run after resetToken. The attach logic should be moved into the onTokenChange success path to ensure it only runs when a valid token exists and can re-attach after token reset.

Applied to files:

• dashboard/backend/public/ensure-cloud-token.ts

📚 Learning: 2025-09-09T20:13:36.836Z

Learnt from: mabels
Repo: fireproof-storage/fireproof PR: 1130
File: core/gateways/cloud/to-cloud.ts:155-160
Timestamp: 2025-09-09T20:13:36.836Z
Learning: In core/gateways/cloud/to-cloud.ts, the configHash method currently hashes this.opts directly, which includes volatile fields like strategy/token/events/context. The user (mabels) indicated this is part of a second iteration approach, suggesting they plan to address hash stability in a future iteration.

Applied to files:

• dashboard/backend/public/ensure-cloud-token.ts

📚 Learning: 2025-09-09T19:56:31.545Z

Learnt from: mabels
Repo: fireproof-storage/fireproof PR: 1130
File: core/device-id/device-id-protocol.ts:47-60
Timestamp: 2025-09-09T19:56:31.545Z
Learning: In the Result type from adviser/cement, Result.Err can accept Result objects directly without needing to unwrap them with .Err(). The codebase consistently uses Result.Err(resultObject) pattern, as seen in core/keybag/key-bag.ts lines 94 and 102.

Applied to files:

• dashboard/backend/public/ensure-cloud-token.ts

🧬 Code graph analysis (1)

dashboard/backend/public/ensure-cloud-token.ts (6)

dashboard/backend/types.ts (1)

• FPApiSQLCtx (94-100)

core/protocols/dashboard/msg-types.ts (2)

• ReqEnsureCloudToken (523-533)
• ResEnsureUser (459-463)

dashboard/backend/sql/app-id-bind.ts (1)

• sqlAppIdBinding (5-19)

dashboard/backend/sql/ledgers.ts (1)

• sqlLedgers (7-25)

dashboard/backend/api.ts (1)

• ensureUser (103-105)

dashboard/backend/public/ensure-user.ts (1)

• ensureUser (15-133)

🪛 GitHub Actions: @fireproof/dashboard

dashboard/backend/public/ensure-cloud-token.ts

[error] 1-1: Prettier formatting check failed. File has formatting issues (public/ensure-cloud-token.ts). Run 'prettier --write' to fix the code style.

🔇 Additional comments (4)

dashboard/backend/tests/db-api.test.ts (1)

1202-1235: Well-structured regression test for the new invite policy.

The test correctly validates that an uninvited user (User C) cannot access an appId already bound by another user (User A). The test setup is isolated with a unique appId, and the assertions properly verify both the error state and the error message content.

dashboard/backend/public/ensure-cloud-token.ts (3)

35-46: getExistingAppIdBinding helper looks correct.

This helper queries for an existing appId binding without requiring a user association (via sqlLedgerUsers), allowing detection of bindings owned by other users. The query structure correctly joins sqlAppIdBinding with sqlLedgers and applies the appId/env filters.

---

61-75: Good memoization pattern for ensureUser.

The ensureUserOnce closure correctly caches the ensureUser result to avoid redundant calls within a single invocation. This is particularly important because ensureUser includes invite redeem logic that may change binding state, and the subsequent re-fetch at line 106 properly handles this.

---

106-117: Correct implementation of the invite policy enforcement.

The flow properly handles the auto-redeem case:

1. After ensureUserOnce() (which may redeem pending invites), re-fetch the binding to check if access was granted.
2. If still no access, check for an existing binding owned by another user via getExistingAppIdBinding.
3. If such binding exists, block the request with a clear invite guidance message.

This aligns with the PR objective to block uninvited users while preserving the auto-redeem flow.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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