Skip to content

feat: shared ledger access for invited users #1467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jchris wants to merge 15 commits into
base: main
Choose a base branch
from
+569 −77
Draft

feat: shared ledger access for invited users #1467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
badbaed
feat: add prefix matching for shared ledger access
jchris Dec 15, 2025
f60369e
fix: remove userId suffix from ledger name and broken prefix match
jchris Dec 15, 2025
ae60716
fix: queryCondition now matches by email OR userId
jchris Dec 15, 2025
ad39b11
fix: handle duplicate ledger user gracefully with onConflictDoNothing
jchris Dec 15, 2025
448e6cf
fix: check for existing AppIdBinding before creating new ledger
jchris Dec 15, 2025
282e47d
fix: enforce strict policy - verify user in LedgerUsers before issuin…
jchris Dec 15, 2025
f725f5b
test: add strict policy tests for ensureCloudToken
jchris Dec 15, 2025
48165a8
fix: auto-redeem invites in ensureCloudToken via ensureUser call
jchris Dec 16, 2025
ad0e56e
fix: ensureCloudToken creates user first for new invited users
jchris Dec 16, 2025
d2555e6
chore: add p0.46 version logging to ensureCloudToken
jchris Dec 16, 2025
4b64c28
chore: add p0.47 detailed logging to redeemInvite
jchris Dec 16, 2025
dd3d430
style: format redeem-invite.ts
jchris Dec 16, 2025
274d08e
Add p0.48 logging to inviteUser for debugging invite creation
jchris Dec 16, 2025
8970ac8
Add p0.49 logging to ensureCloudToken for appId binding debug
jchris Dec 16, 2025
abc0e81
style: format ensure-cloud-token.ts
jchris Dec 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion dashboard/backend/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,16 @@ export class FPApiSQL implements FPApiInterface {
getCertFromCsr(req: ReqCertFromCsr): Promise<Result<ResCertFromCsr>> {
return this.#checkAuth(req, (req) => getCertFromCsr(this.ctx, req));
}
ensureCloudToken(req: ReqEnsureCloudToken, ictx: Partial<FPTokenContext> = {}): Promise<Result<ResEnsureCloudToken>> {
async ensureCloudToken(req: ReqEnsureCloudToken, ictx: Partial<FPTokenContext> = {}): Promise<Result<ResEnsureCloudToken>> {
// p0.46 - For new users, ensureUser must be called first to create them
// This allows invited users to access shared ledgers on first visit
console.log("[ensureCloudToken p0.46] starting for appId:", req.appId);
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 16, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove PII logging before production deployment.

User IDs are logged to console (lines 163, 166), which may violate privacy regulations depending on how user IDs are classified in your system. Consider removing or redacting this PII.

Apply this diff:

-    console.log("[ensureCloudToken p0.46] starting for appId:", req.appId);
+    console.log("[ensureCloudToken p0.46] starting");
     const rUser = await ensureUser(this.ctx, { type: "reqEnsureUser", auth: req.auth });
     if (rUser.isErr()) {
       console.log("[ensureCloudToken p0.46] ensureUser failed:", rUser.Err());
       return Result.Err(rUser.Err());
     }
-    console.log("[ensureCloudToken p0.46] ensureUser succeeded, userId:", rUser.Ok().user.userId);
+    console.log("[ensureCloudToken p0.46] ensureUser succeeded");

Also applies to: 163-163, 166-166

🤖 Prompt for AI Agents 
In dashboard/backend/api.ts around lines 160, 163 and 166, the code currently
logs user/app identifiers to the console which may contain PII; remove or redact
those console.log calls and replace them with non-PII-safe logging.
Specifically, delete or comment out the console.log that prints req.appId and
any other user ID values, or replace them with a logger call that only emits
non-identifying context (e.g., operation names, truncated/redacted IDs like
first 4 chars + “…” or a hash), and ensure no raw user identifiers are written
to stdout in these lines before production deployment.

const rUser = await ensureUser(this.ctx, { type: "reqEnsureUser", auth: req.auth });
if (rUser.isErr()) {
console.log("[ensureCloudToken p0.46] ensureUser failed:", rUser.Err());
return Result.Err(rUser.Err());
}
console.log("[ensureCloudToken p0.46] ensureUser succeeded, userId:", rUser.Ok().user.userId);
return this.#checkAuth(req, (req) => ensureCloudToken(this.ctx, req, ictx));
}

Expand Down
55 changes: 39 additions & 16 deletions dashboard/backend/internal/add-user-to-ledger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,23 +63,46 @@ export async function addUserToLedger(ctx: FPApiSQLCtx, req: AddUserToLedger): P
.where(and(eq(sqlLedgerUsers.userId, req.userId), ne(sqlLedgerUsers.default, 0)))
.run();
}
const ret = (
await ctx.db
.insert(sqlLedgerUsers)
.values({
ledgerId: ledger.Ledgers.ledgerId,
userId: req.userId,
name: req.userName,
role: req.role,
right: req.right,
default: req.default ? 1 : 0,
createdAt: now,
updatedAt: now,
})
.returning()
)[0];
const inserted = await ctx.db
.insert(sqlLedgerUsers)
.values({
ledgerId: ledger.Ledgers.ledgerId,
userId: req.userId,
name: req.userName,
role: req.role,
right: req.right,
default: req.default ? 1 : 0,
createdAt: now,
updatedAt: now,
})
.onConflictDoNothing()
.returning();
if (inserted.length === 0) {
// User already exists in ledger, fetch existing record
const existing = await ctx.db
.select()
.from(sqlLedgerUsers)
.where(and(eq(sqlLedgerUsers.ledgerId, ledger.Ledgers.ledgerId), eq(sqlLedgerUsers.userId, req.userId)))
.get();
if (!existing) {
return Result.Err("failed to insert or find ledger user");
}
return Result.Ok({
ledgerName: toUndef(ledger.Ledgers.name),
userName: toUndef(existing.name),
ledgerId: ledger.Ledgers.ledgerId,
tenantId: ledger.Ledgers.tenantId,
status: existing.status as UserStatus,
statusReason: existing.statusReason,
userId: req.userId,
default: !!existing.default,
role: toRole(existing.role),
right: toReadWrite(existing.right),
});
}
Comment on lines +80 to +102
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 15, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix: Query must filter by role and status to match primary key.

The conflict case query filters only by ledgerId and userId, but the primary key is (ledgerId, userId, role). This means a user can have multiple roles for the same ledger, and the query could return the wrong role record. Additionally, the early-return path (line 32) filters by status = "active", but this conflict path doesn't, creating an inconsistency.

Apply this diff to filter by the correct role and status:

    const existing = await ctx.db
      .select()
      .from(sqlLedgerUsers)
-     .where(and(eq(sqlLedgerUsers.ledgerId, ledger.Ledgers.ledgerId), eq(sqlLedgerUsers.userId, req.userId)))
+     .where(
+       and(
+         eq(sqlLedgerUsers.ledgerId, ledger.Ledgers.ledgerId),
+         eq(sqlLedgerUsers.userId, req.userId),
+         eq(sqlLedgerUsers.role, req.role),
+         eq(sqlLedgerUsers.status, "active")
+       )
+     )
      .get();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (inserted.length === 0) {
// User already exists in ledger, fetch existing record
const existing = await ctx.db
.select()
.from(sqlLedgerUsers)
.where(and(eq(sqlLedgerUsers.ledgerId, ledger.Ledgers.ledgerId), eq(sqlLedgerUsers.userId, req.userId)))
.get();
if (!existing) {
return Result.Err("failed to insert or find ledger user");
}
return Result.Ok({
ledgerName: toUndef(ledger.Ledgers.name),
userName: toUndef(existing.name),
ledgerId: ledger.Ledgers.ledgerId,
tenantId: ledger.Ledgers.tenantId,
status: existing.status as UserStatus,
statusReason: existing.statusReason,
userId: req.userId,
default: !!existing.default,
role: toRole(existing.role),
right: toReadWrite(existing.right),
});
}
if (inserted.length === 0) {
// User already exists in ledger, fetch existing record
const existing = await ctx.db
.select()
.from(sqlLedgerUsers)
.where(
and(
eq(sqlLedgerUsers.ledgerId, ledger.Ledgers.ledgerId),
eq(sqlLedgerUsers.userId, req.userId),
eq(sqlLedgerUsers.role, req.role),
eq(sqlLedgerUsers.status, "active")
)
)
.get();
if (!existing) {
return Result.Err("failed to insert or find ledger user");
}
return Result.Ok({
ledgerName: toUndef(ledger.Ledgers.name),
userName: toUndef(existing.name),
ledgerId: ledger.Ledgers.ledgerId,
tenantId: ledger.Ledgers.tenantId,
status: existing.status as UserStatus,
statusReason: existing.statusReason,
userId: req.userId,
default: !!existing.default,
role: toRole(existing.role),
right: toReadWrite(existing.right),
});
}
🤖 Prompt for AI Agents 
In dashboard/backend/internal/add-user-to-ledger.ts around lines 80 to 102, the
conflict-case SELECT only filters by ledgerId and userId but the table's primary
key includes role and the earlier success path required status = "active";
update the .where(...) clause to also filter by the role (use the incoming role
value used when inserting, e.g., req.role or the same variable used for the
insert) and by status = "active" so the query returns the exact role record and
matches the early-return behavior.

⚠️ Potential issue | 🟠 Major

Fix: Respect req.default when returning existing record.

When req.default is true, lines 56-65 clear the default flag on all user's ledger associations (including the conflicting one). However, the conflict path then returns the existing record's default value as-is, which will be 0 after the update, even though the user intended to set this ledger as their default.

Consider updating the existing record to set the default flag when req.default is true:

    if (!existing) {
      return Result.Err("failed to insert or find ledger user");
    }
+   // Update default flag if requested
+   if (req.default && !existing.default) {
+     await ctx.db
+       .update(sqlLedgerUsers)
+       .set({ default: 1, updatedAt: now })
+       .where(
+         and(
+           eq(sqlLedgerUsers.ledgerId, ledger.Ledgers.ledgerId),
+           eq(sqlLedgerUsers.userId, req.userId),
+           eq(sqlLedgerUsers.role, req.role)
+         )
+       )
+       .run();
+   }
    return Result.Ok({
      ledgerName: toUndef(ledger.Ledgers.name),
      userName: toUndef(existing.name),
      ledgerId: ledger.Ledgers.ledgerId,
      tenantId: ledger.Ledgers.tenantId,
      status: existing.status as UserStatus,
      statusReason: existing.statusReason,
      userId: req.userId,
-     default: !!existing.default,
+     default: req.default ?? !!existing.default,
      role: toRole(existing.role),
      right: toReadWrite(existing.right),
    });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (inserted.length === 0) {
// User already exists in ledger, fetch existing record
const existing = await ctx.db
.select()
.from(sqlLedgerUsers)
.where(and(eq(sqlLedgerUsers.ledgerId, ledger.Ledgers.ledgerId), eq(sqlLedgerUsers.userId, req.userId)))
.get();
if (!existing) {
return Result.Err("failed to insert or find ledger user");
}
return Result.Ok({
ledgerName: toUndef(ledger.Ledgers.name),
userName: toUndef(existing.name),
ledgerId: ledger.Ledgers.ledgerId,
tenantId: ledger.Ledgers.tenantId,
status: existing.status as UserStatus,
statusReason: existing.statusReason,
userId: req.userId,
default: !!existing.default,
role: toRole(existing.role),
right: toReadWrite(existing.right),
});
}
if (inserted.length === 0) {
// User already exists in ledger, fetch existing record
const existing = await ctx.db
.select()
.from(sqlLedgerUsers)
.where(and(eq(sqlLedgerUsers.ledgerId, ledger.Ledgers.ledgerId), eq(sqlLedgerUsers.userId, req.userId)))
.get();
if (!existing) {
return Result.Err("failed to insert or find ledger user");
}
// Update default flag if requested
if (req.default && !existing.default) {
await ctx.db
.update(sqlLedgerUsers)
.set({ default: 1, updatedAt: now })
.where(
and(
eq(sqlLedgerUsers.ledgerId, ledger.Ledgers.ledgerId),
eq(sqlLedgerUsers.userId, req.userId),
eq(sqlLedgerUsers.role, req.role)
)
)
.run();
}
return Result.Ok({
ledgerName: toUndef(ledger.Ledgers.name),
userName: toUndef(existing.name),
ledgerId: ledger.Ledgers.ledgerId,
tenantId: ledger.Ledgers.tenantId,
status: existing.status as UserStatus,
statusReason: existing.statusReason,
userId: req.userId,
default: req.default ?? !!existing.default,
role: toRole(existing.role),
right: toReadWrite(existing.right),
});
}
🤖 Prompt for AI Agents 
In dashboard/backend/internal/add-user-to-ledger.ts around lines 80 to 102, the
conflict path returns the existing ledger-user row without applying req.default,
so when req.default is true the prior clear-default update can leave this row as
non-default; modify the conflict branch to, when req.default is true, perform an
update on the existing record to set default = 1 (or true) and persist that
change before returning, ensuring you honor the same transaction/context and
re-read or use the updated values for the returned payload (update status fields
only if necessary, otherwise preserve existing values).

const ret = inserted[0];
return Result.Ok({
ledgerName: ledger.Ledgers.name,
ledgerName: toUndef(ledger.Ledgers.name),
userName: req.userName,
ledgerId: ledger.Ledgers.ledgerId,
tenantId: ledger.Ledgers.tenantId,
Expand Down
107 changes: 77 additions & 30 deletions dashboard/backend/public/ensure-cloud-token.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,39 +88,86 @@ export async function ensureCloudToken(
return Result.Err(`no tenant found for binding of appId:${req.appId} userId:${req.auth.user.userId}`);
}
if (!ledgerId) {
// create ledger
const rCreateLedger = await createLedger(ctx, {
type: "reqCreateLedger",
auth: req.auth,
ledger: {
tenantId,
name: `${req.appId}-${req.auth.user.userId}`,
},
});
if (rCreateLedger.isErr()) {
return Result.Err(rCreateLedger.Err());
}
ledgerId = rCreateLedger.Ok().ledger.ledgerId;
const maxBindings = await ctx.db
.select({
total: count(sqlAppIdBinding.appId),
})
// Check if there's already an AppIdBinding for this appId (without user join)
// This handles the case where an invited user visits before being added to the ledger
const existingBinding = await ctx.db
.select()
.from(sqlAppIdBinding)
.where(eq(sqlAppIdBinding.appId, req.appId))
.innerJoin(sqlLedgers, eq(sqlLedgers.ledgerId, sqlAppIdBinding.ledgerId))
.where(and(eq(sqlAppIdBinding.appId, req.appId), eq(sqlAppIdBinding.env, req.env ?? "prod")))
.get();
Comment on lines +91 to 98
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 16, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Implementation uses exact matching, not prefix matching as described

The PR title and description emphasize "prefix matching" to find ledgers whose names start with the appId (e.g., matching vf-my-app-install1-todos-{userId}), but this implementation uses exact equality on AppIdBinding.appId:

.where(and(eq(sqlAppIdBinding.appId, req.appId), eq(sqlAppIdBinding.env, req.env ?? "prod")))

There's no like() operator or prefix matching here. The query finds an AppIdBinding where appId exactly equals req.appId. This means:

  • If this is correct: The PR description and test comments should be updated to describe "exact matching on appId" rather than "prefix matching on ledger names"
  • If prefix matching was intended: The implementation needs to be changed to query sqlLedgers.name using a LIKE pattern (e.g., like(sqlLedgers.name, \${req.appId}%`)`) as described in the PR objectives

Please clarify which approach is correct. Note that exact matching on AppIdBinding.appId is simpler and avoids the edge cases mentioned in past reviews (multiple matches, LIKE escaping, etc.), but it requires the AppIdBinding table to be properly populated for invited users.

🤖 Prompt for AI Agents 
In dashboard/backend/public/ensure-cloud-token.ts around lines 91 to 98, the
code currently uses exact equality on AppIdBinding.appId but the PR description
states prefix matching on ledger names; decide which behavior you want and
implement accordingly: if you want prefix matching, change the query to join
sqlLedgers and use a LIKE on sqlLedgers.name with a prefix pattern (e.g.,
`${req.appId}%`) and keep the env filter; if exact matching is intended, update
the PR title/description/tests to state exact appId matching and verify
AppIdBinding is populated for invited users. Ensure proper escaping for LIKE
patterns if using prefix matching and keep only the relevant filter (env +
prefix) to avoid incorrect exact-equality logic.

if (maxBindings && maxBindings.total >= ctx.params.maxAppIdBindings) {
return Result.Err(`max appId bindings reached for appId:${req.appId}`);
if (existingBinding) {
console.log(
"[ensureCloudToken p0.49] existingBinding found for appId:",
req.appId,
"ledgerId:",
existingBinding.Ledgers.ledgerId,
);
// Auto-redeem any pending invites for this user (ensureUser calls redeemInvite)
await ensureUser(ctx, {
type: "reqEnsureUser",
auth: req.auth.verifiedAuth,
});
// Verify the user is actually in LedgerUsers before granting access (strict policy)
const userInLedger = await ctx.db
.select()
.from(sqlLedgerUsers)
.where(
and(
eq(sqlLedgerUsers.ledgerId, existingBinding.Ledgers.ledgerId),
eq(sqlLedgerUsers.userId, req.auth.user.userId),
eq(sqlLedgerUsers.status, "active"),
),
)
.get();
console.log(
"[ensureCloudToken p0.49] userInLedger check for ledgerId:",
existingBinding.Ledgers.ledgerId,
"userId:",
req.auth.user.userId,
"found:",
!!userInLedger,
);
if (!userInLedger) {
return Result.Err(`user not authorized for ledger - please accept the invite first`);
}
ledgerId = existingBinding.Ledgers.ledgerId;
tenantId = existingBinding.Ledgers.tenantId;
} else {
// create ledger
const rCreateLedger = await createLedger(ctx, {
type: "reqCreateLedger",
auth: req.auth,
ledger: {
tenantId,
name: req.appId,
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 16, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the file context around line 129
cd dashboard/backend && cat -n public/ensure-cloud-token.ts | sed -n '115,140p'

Repository: fireproof-storage/fireproof

Length of output: 1084

🏁 Script executed:

# Search for PR description or objectives in common locations
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "*.yml" -o -name "*.yaml" \) | head -20

Repository: fireproof-storage/fireproof

Length of output: 661

🏁 Script executed:

# Search for other ledger creation patterns in the codebase
rg "name:\s*req\.appId|name:\s*\`.*appId.*userId" --type ts --type tsx

Repository: fireproof-storage/fireproof

Length of output: 98

🏁 Script executed:

# Search for Ledgers table schema definition
rg "Ledgers.*table|CREATE.*TABLE.*Ledgers" --type ts --type tsx --type sql -A 10

Repository: fireproof-storage/fireproof

Length of output: 98

🏁 Script executed:

# Search for how ledger naming is handled elsewhere in the codebase
rg "appId.*ownerUserId|ownerUserId.*appId" --type ts --type tsx -B 2 -A 2

Repository: fireproof-storage/fireproof

Length of output: 98

🏁 Script executed:

# Let me adjust the search - remove tsx type and search more broadly
rg "name.*appId" --type ts -B 2 -A 2

Repository: fireproof-storage/fireproof

Length of output: 1609

🏁 Script executed:

# Search for other createLedger calls to see naming patterns
rg "createLedger" --type ts -B 3 -A 3

Repository: fireproof-storage/fireproof

Length of output: 22537

🏁 Script executed:

# Search for ownerUserId or userId in ledger context
rg "ownerUserId|userId.*ledger|ledger.*userId" --type ts -B 1 -A 1

Repository: fireproof-storage/fireproof

Length of output: 9109

🏁 Script executed:

# Look for PR description in the repository (check git logs or PR-related files)
git log --oneline --all | head -20

Repository: fireproof-storage/fireproof

Length of output: 228

🏁 Script executed:

# Check if there's a specific PR or branch with objectives
git branch -a | grep -E "801|enhance|ledger" || echo "No matching branches found"

Repository: fireproof-storage/fireproof

Length of output: 97

🏁 Script executed:

# Look at the test section about ledger naming
sed -n '1,150p' dashboard/backend/tests/db-api.test.ts | grep -A 40 "appId.*ownerUserId"

Repository: fireproof-storage/fireproof

Length of output: 53

🏁 Script executed:

# Look for the actual test implementation that uses appId-ownerUserId format
rg "appId.*userId|userId.*appId" dashboard/backend/tests/db-api.test.ts -B 5 -A 5

Repository: fireproof-storage/fireproof

Length of output: 53

🏁 Script executed:

# Check if there are any TODOs or comments about ledger naming in ensure-cloud-token.ts
cat -n dashboard/backend/public/ensure-cloud-token.ts | sed -n '100,160p'

Repository: fireproof-storage/fireproof

Length of output: 2579

🏁 Script executed:

# Look for all places where ledger names are created with appId
rg "name:\s*\`?\$?\{?req\.appId" dashboard/backend -B 3 -A 3

Repository: fireproof-storage/fireproof

Length of output: 558

🏁 Script executed:

# Read the test section that mentions appId-ownerUserId more carefully
sed -n '1200,1350p' dashboard/backend/tests/db-api.test.ts

Repository: fireproof-storage/fireproof

Length of output: 4960

🏁 Script executed:

# Check the git history of the ensure-cloud-token.ts file to see if there were recent changes
git log --oneline -20 -- dashboard/backend/public/ensure-cloud-token.ts

Repository: fireproof-storage/fireproof

Length of output: 143

🏁 Script executed:

# Look for comments or TODOs about ledger naming in the codebase
rg "TODO|FIXME|XXX|HACK" dashboard/backend/public/ensure-cloud-token.ts dashboard/backend/tests/db-api.test.ts

Repository: fireproof-storage/fireproof

Length of output: 53

🏁 Script executed:

# Search for any documentation on the intended ledger naming scheme
rg "ledger.*name|name.*format" --type md --type ts -B 2 -A 2

Repository: fireproof-storage/fireproof

Length of output: 23497

Ledger naming doesn't match documented format

The test documentation describes ledger names as {appId}-{ownerUserId}, with prefix matching to allow invited users to access shared ledgers. However, the code at line 129 creates ledgers with only the appId:

name: req.appId,

This works for the current binding model (one appId per ledger per tenant), but deviates from the documented design. Either the code should be updated to use name: ${req.appId}-${req.auth.user.userId}`` to match the specification and enable proper prefix-based ledger sharing, or the documentation should be updated to reflect that ledger names are now just appId.

🤖 Prompt for AI Agents 
In dashboard/backend/public/ensure-cloud-token.ts around line 129, the ledger
name is being created as just the appId (name: req.appId) which diverges from
the documented `{appId}-{ownerUserId}` format that supports prefix-based
sharing; update the ledger name creation to concatenate the owner user id (e.g.,
use `${req.appId}-${req.auth.user.userId}`) so ledgers follow the documented
naming and allow invited users to access shared ledgers, or alternatively update
the docs if the intended design is to keep names as just appId—apply the code
change if you want to retain the documented sharing behavior.

},
});
if (rCreateLedger.isErr()) {
return Result.Err(rCreateLedger.Err());
}
ledgerId = rCreateLedger.Ok().ledger.ledgerId;
const maxBindings = await ctx.db
.select({
total: count(sqlAppIdBinding.appId),
})
.from(sqlAppIdBinding)
.where(eq(sqlAppIdBinding.appId, req.appId))
.get();
if (maxBindings && maxBindings.total >= ctx.params.maxAppIdBindings) {
return Result.Err(`max appId bindings reached for appId:${req.appId}`);
}
Comment on lines +150 to +159
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 15, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Check maxAppIdBindings before creating the ledger to avoid orphaned records.

The maxAppIdBindings validation occurs after createLedger has already inserted a new ledger (line 105). If the check at line 124 fails, the function returns an error but leaves the newly created ledger in the database without a corresponding AppIdBinding. This wastes the tenant's ledger quota and violates atomicity.

Move the max-bindings check before ledger creation:

       } else {
+        // Validate binding quota before creating resources
+        const maxBindings = await ctx.db
+          .select({
+            total: count(sqlAppIdBinding.appId),
+          })
+          .from(sqlAppIdBinding)
+          .where(eq(sqlAppIdBinding.appId, req.appId))
+          .get();
+        if (maxBindings && maxBindings.total >= ctx.params.maxAppIdBindings) {
+          return Result.Err(`max appId bindings reached for appId:${req.appId}`);
+        }
+
         // create ledger
         const rCreateLedger = await createLedger(ctx, {
           type: "reqCreateLedger",
           auth: req.auth,
           ledger: {
             tenantId,
             name: req.appId,
           },
         });
         if (rCreateLedger.isErr()) {
           return Result.Err(rCreateLedger.Err());
         }
         ledgerId = rCreateLedger.Ok().ledger.ledgerId;
-        const maxBindings = await ctx.db
-          .select({
-            total: count(sqlAppIdBinding.appId),
-          })
-          .from(sqlAppIdBinding)
-          .where(eq(sqlAppIdBinding.appId, req.appId))
-          .get();
-        if (maxBindings && maxBindings.total >= ctx.params.maxAppIdBindings) {
-          return Result.Err(`max appId bindings reached for appId:${req.appId}`);
-        }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In dashboard/backend/public/ensure-cloud-token.ts around lines 105-126, the
maxAppIdBindings check currently runs after createLedger and can leave an
orphaned ledger; move the database count and the if (maxBindings.total >=
ctx.params.maxAppIdBindings) validation so it executes before calling
createLedger (i.e., perform the select/count and early return on exceeding
maxAppIdBindings immediately prior to ledger creation), or alternatively wrap
both the check, ledger creation and AppIdBinding insert in a single DB
transaction to ensure atomicity so no ledger is created when the binding limit
is exceeded.

await ctx.db
.insert(sqlAppIdBinding)
.values({
appId: req.appId,
env: req.env ?? "prod",
ledgerId,
tenantId,
createdAt: new Date().toISOString(),
})
.run();
}
await ctx.db
.insert(sqlAppIdBinding)
.values({
appId: req.appId,
env: req.env ?? "prod",
ledgerId,
tenantId,
createdAt: new Date().toISOString(),
})
.run();
}
} else {
ledgerId = binding.Ledgers.ledgerId;
Expand Down
4 changes: 3 additions & 1 deletion dashboard/backend/public/ensure-user.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,12 @@ export async function ensureUser(ctx: FPApiSQLCtx, req: ReqEnsureUser): Promise<
}
}
// Auto-redeem any pending invites for this user
await redeemInvite(ctx, {
console.log("[ensureUser p0.47] calling redeemInvite for email:", auth.verifiedAuth.params.email);
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 16, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove PII logging before production deployment.

Email addresses and potentially other user identifiers in the redeemInvite result are being logged to console. This violates privacy regulations (GDPR, CCPA) and best practices.

Remove or redact PII from these logs:

-  console.log("[ensureUser p0.47] calling redeemInvite for email:", auth.verifiedAuth.params.email);
+  console.log("[ensureUser p0.47] calling redeemInvite for user");
   const redeemResult = await redeemInvite(ctx, {
     type: "reqRedeemInvite",
     auth,
   });
-  console.log("[ensureUser p0.47] redeemInvite result:", JSON.stringify(redeemResult));
+  console.log("[ensureUser p0.47] redeemInvite completed, found invites:", redeemResult.isOk() ? redeemResult.Ok().invites.length : 0);

Also applies to: 129-129

🤖 Prompt for AI Agents 
In dashboard/backend/public/ensure-user.ts around lines 124 and 129, remove or
redact PII from the console logs that print auth.verifiedAuth.params.email;
replace the direct email output with a non-identifying token (e.g., masked email
like a****@domain or a hashed/trimmed identifier) or entirely remove the log,
and ensure any remaining logging is behind a debug/verbose flag or environment
check so production never emits user emails.

const redeemResult = await redeemInvite(ctx, {
type: "reqRedeemInvite",
auth,
});
console.log("[ensureUser p0.47] redeemInvite result:", JSON.stringify(redeemResult));
return Result.Ok({
type: "resEnsureUser",
user: user,
Expand Down
35 changes: 35 additions & 0 deletions dashboard/backend/public/invite-user.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,30 @@ import { createInviteTicket } from "../internal/create-invite-ticket.js";
import { updateInviteTicket } from "../internal/update-invite.ticket.js";

export async function inviteUser(ctx: FPApiSQLCtx, req: ReqWithVerifiedAuthUser<ReqInviteUser>): Promise<Result<ResInviteUser>> {
console.log(
"[inviteUser p0.48] starting with request:",
JSON.stringify({
query: req.ticket.query,
invitedParams: req.ticket.invitedParams,
inviteId: req.ticket.inviteId,
}),
);
if (!isVerifiedUserActive(req.auth)) {
console.log("[inviteUser p0.48] user not active");
return Result.Err(new UserNotFoundError());
}
const findUser = await queryUser(ctx.db, req.ticket.query);
if (findUser.isErr()) {
console.log("[inviteUser p0.48] queryUser failed:", findUser.Err());
return Result.Err(findUser.Err());
}
console.log("[inviteUser p0.48] queryUser found:", findUser.Ok().length, "users");
if (req.ticket.query.existingUserId && findUser.Ok().length !== 1) {
console.log("[inviteUser p0.48] existingUserId not found");
return Result.Err("existingUserId not found");
}
if (req.ticket.query.existingUserId === req.auth.user.userId) {
console.log("[inviteUser p0.48] cannot invite self");
return Result.Err("cannot invite self");
}

Expand All @@ -34,38 +47,60 @@ export async function inviteUser(ctx: FPApiSQLCtx, req: ReqWithVerifiedAuthUser<
let tenantId: string | undefined;
let ledgerId: string | undefined;
if (req.ticket.invitedParams?.ledger) {
console.log("[inviteUser p0.48] looking for ledger:", req.ticket.invitedParams.ledger.id);
const ledger = await ctx.db.select().from(sqlLedgers).where(eq(sqlLedgers.ledgerId, req.ticket.invitedParams.ledger.id)).get();
if (!ledger) {
console.log("[inviteUser p0.48] ledger not found");
return Result.Err("ledger not found");
}
ledgerId = ledger.ledgerId;
tenantId = ledger.tenantId;
console.log("[inviteUser p0.48] found ledger:", ledgerId, "tenantId:", tenantId);
}
if (req.ticket.invitedParams?.tenant) {
console.log("[inviteUser p0.48] looking for tenant:", req.ticket.invitedParams.tenant.id);
const tenant = await ctx.db.select().from(sqlTenants).where(eq(sqlTenants.tenantId, req.ticket.invitedParams.tenant.id)).get();
if (!tenant) {
console.log("[inviteUser p0.48] tenant not found");
return Result.Err("tenant not found");
}
tenantId = tenant.tenantId;
console.log("[inviteUser p0.48] found tenant:", tenantId);
}
if (!tenantId) {
console.log("[inviteUser p0.48] no tenant found");
return Result.Err("tenant not found");
}

let inviteTicket: InviteTicket;
if (!req.ticket.inviteId) {
console.log("[inviteUser p0.48] creating new invite for tenantId:", tenantId, "ledgerId:", ledgerId);
const rInviteTicket = await createInviteTicket(ctx, req.auth.user.userId, tenantId, ledgerId, req);
if (rInviteTicket.isErr()) {
console.log("[inviteUser p0.48] createInviteTicket failed:", rInviteTicket.Err());
return Result.Err(rInviteTicket.Err());
}
inviteTicket = rInviteTicket.Ok();
console.log(
"[inviteUser p0.48] created invite:",
JSON.stringify({
inviteId: inviteTicket.inviteId,
ledgerId: inviteTicket.invitedParams.ledger?.id,
tenantId: inviteTicket.invitedParams.tenant?.id,
queryEmail: inviteTicket.query.byEmail,
}),
);
} else {
console.log("[inviteUser p0.48] updating invite:", req.ticket.inviteId);
const rInviteTicket = await updateInviteTicket(ctx, req.auth.user.userId, tenantId, ledgerId, req);
if (rInviteTicket.isErr()) {
console.log("[inviteUser p0.48] updateInviteTicket failed:", rInviteTicket.Err());
return Result.Err(rInviteTicket.Err());
}
inviteTicket = rInviteTicket.Ok();
console.log("[inviteUser p0.48] updated invite:", inviteTicket.inviteId);
}
console.log("[inviteUser p0.48] success, returning invite:", inviteTicket.inviteId);
return Result.Ok({
type: "resInviteUser",
invite: inviteTicket,
Expand Down
38 changes: 27 additions & 11 deletions dashboard/backend/public/redeem-invite.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,37 @@ export async function redeemInvite(
ctx: FPApiSQLCtx,
req: ReqWithVerifiedAuthUser<ReqRedeemInvite>,
): Promise<Result<ResRedeemInvite>> {
const query = {
byString: req.auth.verifiedAuth.params.email,
byNick: req.auth.verifiedAuth.params.nick,
existingUserId: req.auth.user.userId,
};
console.log("[redeemInvite p0.47] searching with query:", JSON.stringify(query));
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 16, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove PII logging and fix formatting issues.

Two issues to address:

  1. Privacy violation: Email addresses and user identifiers are logged to console (lines 21, 25), which violates GDPR/CCPA requirements.
  2. Pipeline failure: Prettier formatting check failed for this file.

Fix PII logging and run pnpm run format --write:

-  console.log("[redeemInvite p0.47] searching with query:", JSON.stringify(query));
+  console.log("[redeemInvite p0.47] searching for invites");
   const foundInvites = await findInvite(ctx, { query });
-  console.log("[redeemInvite p0.47] found invites:", foundInvites.length, "pending:", foundInvites.filter((i) => i.status === "pending").length);
-  if (foundInvites.length > 0) {
-    console.log("[redeemInvite p0.47] invite details:", JSON.stringify(foundInvites.map((i) => ({ id: i.inviteId, status: i.status, queryEmail: i.query.byEmail, ledger: i.invitedParams.ledger?.id }))));
-  }
+  console.log("[redeemInvite p0.47] found invites:", foundInvites.length, "pending:", foundInvites.filter((i) => i.status === "pending").length);

Also applies to: 24-26

🤖 Prompt for AI Agents 
In dashboard/backend/public/redeem-invite.ts around lines 21 and 24-26, the
console.log calls currently print user PII (email and identifiers) and the file
also fails Prettier formatting; remove or redact PII from logs (e.g., log only
non-identifying metadata like "query received" or a hashed/masked id) and
replace JSON.stringify(query) with a safe representation that omits or masks
email/IDs, then run the formatter with `pnpm run format --write` to fix
formatting issues.

const foundInvites = await findInvite(ctx, { query });
console.log(
"[redeemInvite p0.47] found invites:",
foundInvites.length,
"pending:",
foundInvites.filter((i) => i.status === "pending").length,
);
if (foundInvites.length > 0) {
console.log(
"[redeemInvite p0.47] invite details:",
JSON.stringify(
foundInvites.map((i) => ({
id: i.inviteId,
status: i.status,
queryEmail: i.query.byEmail,
ledger: i.invitedParams.ledger?.id,
})),
),
);
}
return Result.Ok({
type: "resRedeemInvite",
invites: sqlToInviteTickets(
await Promise.all(
(
await findInvite(ctx, {
query: {
byString: req.auth.verifiedAuth.params.email,
byNick: req.auth.verifiedAuth.params.nick,
existingUserId: req.auth.user.userId,
// TODO
// andProvider: req.auth.verifiedAuth.provider,
},
})
)
foundInvites
.filter((i) => i.status === "pending")
.map(async (invite) => {
if (invite.invitedParams.tenant) {
Expand Down
Loading
Loading