[codex] Add signed Nostr seller profiles#12
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request introduces seller profile support via Nostr kind 0 events. A new Changes
Sequence DiagramsequenceDiagram
participant Client
participant API as /api/sellers/profile
participant NostrVerify as Signature Verification
participant ProfileParser as Profile Parser
participant DB as Database
participant Response
Client->>API: POST signed profile event (kind 0)
API->>NostrVerify: verifyNostrEvent(event)
alt Signature Invalid
NostrVerify-->>API: verification error
API-->>Client: 422 errorResponse
else Signature Valid
NostrVerify-->>API: verified ✓
API->>ProfileParser: parseSellerProfileContent(event.content)
alt Parse Error
ProfileParser-->>API: validation error
API-->>Client: 400 validationErrorResponse
else Parse Success
ProfileParser-->>API: SellerProfileData
API->>DB: upsertSellerProfileFromEvent(pubkey, event, profile)
Note over DB: Verify supersession logic<br/>Store/update event record<br/>Replace old profile if newer
DB-->>API: seller (updated)
API-->>Client: 200 jsonResponse {seller, profile_updated}
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~85 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (1)
64-72:⚠️ Potential issue | 🟡 MinorPluralize follow-up sentence after "Apply the migrations".
Line 64 now refers to multiple migrations, but line 66 still reads "The migration creates:". Suggest matching the plural:
📝 Doc nit
-The migration creates: +The migrations create:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 64 - 72, The sentence after "Apply the migrations in `supabase/migrations/` in timestamp order." uses singular wording; update the phrase "The migration creates:" to the plural "The migrations create:" so it agrees with "migrations" and the ensuing bullet list (`sellers`, `listings`, `listing_events`, `listing_fee_payments`, `audit_logs`).lib/repository.ts (1)
314-361:⚠️ Potential issue | 🟠 MajorConcurrent first-time seller registration regresses to a unique-constraint error.
This refactor replaces the previous conflict-based
upsertwith an explicit "find-then-update-or-insert" flow. When two requests for the same brand-newpubkeyarrive concurrently (which is realistic given thatupsertSellerProfileFromEventandcreateListingFromEventboth callupsertSeller({ pubkey })), both seeexisting === nulland both attempt theinsertat line 347. The second one fails with the unique constraint onpubkey, andvalidationErrorResponsewill surface that to the client even though the request is perfectly valid.Suggest restoring conflict-tolerant behavior on the insert path:
🛡️ Proposed fix
const { data, error } = await client .from("sellers") - .insert( + .upsert( sellerToRow({ pubkey: input.pubkey, displayName: input.displayName, contactMethodType: input.contactMethodType, contactMethodValue: input.contactMethodValue, walletType: input.walletType, walletMetadata: input.walletMetadata as JsonObject | undefined, }), + { onConflict: "pubkey", ignoreDuplicates: false }, ) .select("*") .single();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/repository.ts` around lines 314 - 361, The current upsertSeller (uses requireClient, getSellerByPubkey, sellerToRow, sellerFromRow) does a find-then-insert which races on concurrent inserts and surfaces a unique-constraint error; fix by making the insert conflict-tolerant: either use the client's upsert/onConflict API (e.g., .insert(...).onConflict('pubkey').merge() or .upsert(...) depending on your client) so the DB will merge/ignore on pubkey conflict and return the row, or wrap the current insert in a try/catch that detects a unique-constraint error on pubkey, then re-query getSellerByPubkey(input.pubkey) and return that row via sellerFromRow; apply this change inside upsertSeller replacing the current insert block.
🧹 Nitpick comments (2)
scripts/freeport-agent.ts (1)
110-124: Optional: route error payload to stderr.
postProfileprints the error body viaconsole.logbeforeprocess.exit(1). This mirrorspostListing, but printing failures to stdout means a--out-style redirect would capture an error envelope as if it were a successful response. Considerconsole.erroron the failure branch. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/freeport-agent.ts` around lines 110 - 124, postProfile currently logs error response bodies to stdout before exiting; change the failure branch to write the error payload to stderr instead. In the postProfile function, replace the console.log used inside the if (!response.ok) block with console.error (matching postListing’s error behaviour) so error envelopes are emitted on stderr while successful responses remain on stdout.scripts/check-profile-flow.tsx (1)
84-106: Consider assertingsupersededByEventIdlinkage to catch repository bugs.The current assertions verify that the older posted event is stored (line 95–96), but don't verify the supersession pointers. Strengthening these checks would close a coverage gap I flagged on
lib/repository.ts(memory implementation never updates the prior current event'ssupersededByEventIdwhen a newer event replaces it):♻️ Suggested additional assertions
const storedOlder = await getRepository().getEvent(older.id); assert.ok(storedOlder); assert.equal(storedOlder.listingId, null); + assert.equal(storedOlder.supersededByEventId, newer.id); @@ assert.equal(latestResponse.body.profile_updated, true); assert.equal(latestResponse.body.seller.profile_display_name, "Latest Profile"); + + const storedNewer = await getRepository().getEvent(newer.id); + assert.ok(storedNewer); + assert.equal(storedNewer.supersededByEventId, latest.id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-profile-flow.tsx` around lines 84 - 106, Add assertions to verify the supersession linkage after posting the newer profile event: after storing the older event (using getRepository().getEvent(older.id) / symbol storedOlder) assert that storedOlder.supersededByEventId equals latest.id, and also fetch the latest event (getRepository().getEvent(latest.id) / symbol storedLatest) and assert storedLatest.supersededByEventId is null (or undefined) to ensure the repository updated the prior event's supersededByEventId and the latest remains un-superseded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/sellers/profile/route.ts`:
- Around line 8-33: The POST handler currently accepts events with unbounded
event.created_at which allows future-dated events to permanently win in
shouldReplaceSellerProfile; before calling verifyNostrEvent (or at least before
calling getRepository().upsertSellerProfileFromEvent), validate and clamp/reject
parsed.event.created_at against the server time (Date.now()/1000) with a small
allowed skew (e.g., a few minutes); if created_at exceeds the allowed future
skew, either return a 422 errorResponse or set created_at to now (and document
that change), so shouldReplaceSellerProfile in lib/repository.ts cannot be
permanently outcompeted by a far-future timestamp.
- Around line 30-32: The catch block in route.ts currently funnels all errors
through validationErrorResponse, hiding operational failures; update the error
handling around parseSellerProfileContent and upsertSellerProfileFromEvent by
distinguishing validation errors (from parseSellerProfileContent or Zod) —
return via validationErrorResponse — from database/network/errors thrown by
upsertSellerProfileFromEvent and other operational code, and return an
appropriate 5xx server error response (e.g., serverErrorResponse or a 500/503
wrapper) so transient failures are not treated as 400; apply the same pattern
where you see this catch-all behavior across other API routes.
In `@app/api/sellers/register/route.ts`:
- Around line 17-22: The registration response omits timestamps because
sellerToPublicJson(seller) is called without the includeTimestamps option;
update the return in the register handler to call sellerToPublicJson(seller, {
includeTimestamps: true }) so the response includes created_at and updated_at
and matches the shape returned by GET /api/sellers/[pubkey] and POST
/api/sellers/profile; ensure any related typing or consumers still accept the
timestamps.
In `@app/listings/`[id]/page.tsx:
- Around line 91-99: The Website link can be a javascript: XSS vector; ensure
the upstream validator for profileWebsite restricts schemes to http/https (e.g.,
pin protocols in your Zod/schema), and add a render-time guard in
app/listings/[id]/page.tsx where label === "Website" (using the value variable)
to only render the <a> if value begins with "http://" or "https://"; otherwise
omit the anchor or render a safe plain text fallback. Also consider normalizing
or rejecting non-HTTP(S) values when saving via the profileWebsite field.
- Around line 65-72: The inline style using pictureUrl in the backgroundImage of
the div (where sellerAvatarInitial(listing.seller) is used as fallback) allows
CSS injection/SSRF via crafted URLs; stop embedding the URL inside style and
instead render a standard image element (e.g., <img> or next/image) with
src={pictureUrl} and the same classes for sizing/rounding, keeping
sellerAvatarInitial(listing.seller) as the visual fallback when pictureUrl is
falsy; remove the style prop and backgroundImage usage entirely so the browser
handles the URL via src parsing rather than CSS string interpolation.
In `@components/listing-card.tsx`:
- Around line 57-63: The inline style using backgroundImage with pictureUrl in
listing-card.tsx (the div that currently uses style={pictureUrl ? {
backgroundImage: `url("${pictureUrl}")` } : undefined}) is vulnerable to URL
injection; replace this pattern by rendering a proper <img> element for the
avatar when pictureUrl exists (fall back to sellerAvatarInitial(listing.seller)
when absent) or, if you must keep background images, strictly validate/sanitize
pictureUrl (e.g., parse with the URL constructor, enforce http(s) scheme and a
hostname allowlist) before assigning it to the style; update the component that
references pictureUrl and sellerAvatarInitial accordingly.
In `@lib/repository.ts`:
- Around line 145-172: In upsertSellerProfileFromEvent, when a new profile
replaces the current one you must patch the previously-stored event's
supersededByEventId in the in-memory store just like the Supabase path does;
after computing profileUpdated and before pushing/returning, locate the existing
event record in this.store.events whose eventId equals seller.profileEventId
(the prior current event) and set its supersededByEventId = event.id (and if you
already set eventRecord.supersededByEventId for the incoming record, only do
that when the incoming event is older), so that
seller.profileEventId/seller.profileEventCreatedAt are updated and the previous
event reflects it in memory.
- Around line 363-410: upsertSellerProfileFromEvent has a TOCTOU:
shouldReplaceSellerProfile is decided from the in-memory seller snapshot but the
.update on the "sellers" row is unconditional, so a concurrently-committed newer
profile can be overwritten; change the update to be conditional in SQL (e.g.,
add a .eq("id", seller.id).lt("profile_event_created_at", event.created_at) or
compare profile_event_id/profile_event_created_at in the WHERE) so the DB only
accepts the update if the stored profile is older than the incoming event; do
the same conditional check before updating listing_events.superseded_by_event_id
(only mark the previous event as superseded if you successfully updated the
seller row), and optionally guard or serialize using pg_advisory_xact_lock or a
DB-side stored procedure/unique constraint if you prefer DB-enforced ordering.
---
Outside diff comments:
In `@lib/repository.ts`:
- Around line 314-361: The current upsertSeller (uses requireClient,
getSellerByPubkey, sellerToRow, sellerFromRow) does a find-then-insert which
races on concurrent inserts and surfaces a unique-constraint error; fix by
making the insert conflict-tolerant: either use the client's upsert/onConflict
API (e.g., .insert(...).onConflict('pubkey').merge() or .upsert(...) depending
on your client) so the DB will merge/ignore on pubkey conflict and return the
row, or wrap the current insert in a try/catch that detects a unique-constraint
error on pubkey, then re-query getSellerByPubkey(input.pubkey) and return that
row via sellerFromRow; apply this change inside upsertSeller replacing the
current insert block.
In `@README.md`:
- Around line 64-72: The sentence after "Apply the migrations in
`supabase/migrations/` in timestamp order." uses singular wording; update the
phrase "The migration creates:" to the plural "The migrations create:" so it
agrees with "migrations" and the ensuing bullet list (`sellers`, `listings`,
`listing_events`, `listing_fee_payments`, `audit_logs`).
---
Nitpick comments:
In `@scripts/check-profile-flow.tsx`:
- Around line 84-106: Add assertions to verify the supersession linkage after
posting the newer profile event: after storing the older event (using
getRepository().getEvent(older.id) / symbol storedOlder) assert that
storedOlder.supersededByEventId equals latest.id, and also fetch the latest
event (getRepository().getEvent(latest.id) / symbol storedLatest) and assert
storedLatest.supersededByEventId is null (or undefined) to ensure the repository
updated the prior event's supersededByEventId and the latest remains
un-superseded.
In `@scripts/freeport-agent.ts`:
- Around line 110-124: postProfile currently logs error response bodies to
stdout before exiting; change the failure branch to write the error payload to
stderr instead. In the postProfile function, replace the console.log used inside
the if (!response.ok) block with console.error (matching postListing’s error
behaviour) so error envelopes are emitted on stderr while successful responses
remain on stdout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 00df6763-1bff-45ab-ab30-a069fee127ae
📒 Files selected for processing (23)
README.mdapp/api/sellers/[pubkey]/route.tsapp/api/sellers/profile/route.tsapp/api/sellers/register/route.tsapp/docs/agents/page.tsxapp/docs/api/page.tsxapp/docs/examples/page.tsxapp/listings/[id]/page.tsxcomponents/listing-card.tsxexamples/profile.jsonlib/constants.tslib/db-mappers.tslib/demo-data.tslib/event-mapping.tslib/repository.tslib/seller-profile.tslib/types.tslib/validation.tspackage.jsonpublic/llms.txtscripts/check-profile-flow.tsxscripts/freeport-agent.tssupabase/migrations/20260426000000_add_signed_seller_profiles.sql
| export async function POST(request: Request) { | ||
| try { | ||
| const parsed = SellerProfileRequestSchema.parse(await readJson(request)); | ||
| const verification = verifyNostrEvent(parsed.event); | ||
|
|
||
| if (!verification.ok) { | ||
| return errorResponse( | ||
| { | ||
| code: verification.code, | ||
| message: verification.message, | ||
| }, | ||
| 422, | ||
| ); | ||
| } | ||
|
|
||
| const profile = parseSellerProfileContent(parsed.event.content); | ||
| const result = await getRepository().upsertSellerProfileFromEvent(parsed.event, profile); | ||
|
|
||
| return jsonResponse({ | ||
| seller: sellerToPublicJson(result.seller, { includeTimestamps: true }), | ||
| profile_updated: result.profileUpdated, | ||
| }); | ||
| } catch (error) { | ||
| return validationErrorResponse(error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Future-dated created_at can permanently lock a seller's profile.
shouldReplaceSellerProfile (lib/repository.ts) selects the winning profile event purely by event.created_at (with event.id as tiebreaker). Because there is no upper bound on created_at here, a seller (or anyone holding the key briefly, e.g., a leaked-then-rotated key) can sign a single profile with created_at set far in the future (say year 2999) and from that point forward no honestly-timestamped event will ever supersede it. NIP-01 implementations typically clamp/reject events whose created_at is more than a small delta past Date.now() / 1000.
Consider rejecting (or at least clamping) events whose created_at is meaningfully greater than the current server time, e.g.:
🛡️ Suggested guard before signature verification
const parsed = SellerProfileRequestSchema.parse(await readJson(request));
+ const nowSeconds = Math.floor(Date.now() / 1000);
+ if (parsed.event.created_at > nowSeconds + 15 * 60) {
+ return errorResponse(
+ {
+ code: "event_created_at_in_future",
+ message: "Event created_at is too far in the future.",
+ },
+ 422,
+ );
+ }
const verification = verifyNostrEvent(parsed.event);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/sellers/profile/route.ts` around lines 8 - 33, The POST handler
currently accepts events with unbounded event.created_at which allows
future-dated events to permanently win in shouldReplaceSellerProfile; before
calling verifyNostrEvent (or at least before calling
getRepository().upsertSellerProfileFromEvent), validate and clamp/reject
parsed.event.created_at against the server time (Date.now()/1000) with a small
allowed skew (e.g., a few minutes); if created_at exceeds the allowed future
skew, either return a 422 errorResponse or set created_at to now (and document
that change), so shouldReplaceSellerProfile in lib/repository.ts cannot be
permanently outcompeted by a far-future timestamp.
| } catch (error) { | ||
| return validationErrorResponse(error); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect validationErrorResponse to see how non-Zod errors are mapped.
fd -t f 'api.ts' lib | xargs -I{} sed -n '1,250p' {}Repository: ATLBitLab/freeport
Length of output: 1460
🏁 Script executed:
cat -n app/api/sellers/profile/route.tsRepository: ATLBitLab/freeport
Length of output: 1435
🏁 Script executed:
# Find parseSellerProfileContent to see what errors it throws
fd -t f -e ts -e js | xargs grep -l "parseSellerProfileContent" | head -5Repository: ATLBitLab/freeport
Length of output: 117
🏁 Script executed:
# Look for other error handling patterns in the codebase for DB/network errors
fd -t f -e ts lib/api*.ts app/api | xargs grep -A 3 "catch.*error" | head -50Repository: ATLBitLab/freeport
Length of output: 44
🏁 Script executed:
cat -n lib/seller-profile.tsRepository: ATLBitLab/freeport
Length of output: 4280
🏁 Script executed:
# Check what verifyNostrEvent throws
rg -A 10 "function verifyNostrEvent" lib/Repository: ATLBitLab/freeport
Length of output: 510
🏁 Script executed:
# Check other API routes to see error handling patterns
fd -t f 'route.ts' app/api | xargs -I{} sh -c 'echo "=== {} ===" && cat -n {} | grep -A 5 "catch"'Repository: ATLBitLab/freeport
Length of output: 1584
Return appropriate HTTP status codes for database errors instead of routing them as 400 validation errors.
The single catch block catches both validation errors from parseSellerProfileContent (plain Error) and database/network errors from upsertSellerProfileFromEvent. Both are routed through validationErrorResponse, which returns 400 for non-Zod errors. This masks transient DB failures as bad_request, breaking retry semantics—HTTP clients and load balancers treat 4xx as non-retryable, while 5xx errors trigger proper retry logic. Consider separating validation errors (return via validationErrorResponse) from operational failures (return 5xx).
This pattern is present across multiple API routes in the codebase and should be addressed systematically.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/sellers/profile/route.ts` around lines 30 - 32, The catch block in
route.ts currently funnels all errors through validationErrorResponse, hiding
operational failures; update the error handling around parseSellerProfileContent
and upsertSellerProfileFromEvent by distinguishing validation errors (from
parseSellerProfileContent or Zod) — return via validationErrorResponse — from
database/network/errors thrown by upsertSellerProfileFromEvent and other
operational code, and return an appropriate 5xx server error response (e.g.,
serverErrorResponse or a 500/503 wrapper) so transient failures are not treated
as 400; apply the same pattern where you see this catch-all behavior across
other API routes.
| return jsonResponse( | ||
| { | ||
| seller: { | ||
| id: seller.id, | ||
| pubkey: seller.pubkey, | ||
| display_name: seller.displayName, | ||
| contact_method_type: seller.contactMethodType, | ||
| contact_method_value: seller.contactMethodValue, | ||
| wallet_type: seller.walletType, | ||
| status: seller.status, | ||
| }, | ||
| seller: sellerToPublicJson(seller), | ||
| }, | ||
| { status: 201 }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm callers of /api/sellers/register don't depend on created_at/updated_at; cross-check sibling routes' option usage.
rg -nP -C3 'sellerToPublicJson\s*\(' --type=ts
rg -nP -C3 '/api/sellers/register' --type=ts --type=tsx --type=mdRepository: ATLBitLab/freeport
Length of output: 2282
Inconsistent timestamps in seller registration response.
The registration endpoint calls sellerToPublicJson(seller) without { includeTimestamps: true }, omitting created_at and updated_at. Both sibling endpoints (GET /api/sellers/[pubkey] and POST /api/sellers/profile) include these timestamps, creating an inconsistent response shape across seller endpoints.
🔧 Proposed fix
- seller: sellerToPublicJson(seller),
+ seller: sellerToPublicJson(seller, { includeTimestamps: true }),📝 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.
| return jsonResponse( | |
| { | |
| seller: { | |
| id: seller.id, | |
| pubkey: seller.pubkey, | |
| display_name: seller.displayName, | |
| contact_method_type: seller.contactMethodType, | |
| contact_method_value: seller.contactMethodValue, | |
| wallet_type: seller.walletType, | |
| status: seller.status, | |
| }, | |
| seller: sellerToPublicJson(seller), | |
| }, | |
| { status: 201 }, | |
| ); | |
| return jsonResponse( | |
| { | |
| seller: sellerToPublicJson(seller, { includeTimestamps: true }), | |
| }, | |
| { status: 201 }, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/sellers/register/route.ts` around lines 17 - 22, The registration
response omits timestamps because sellerToPublicJson(seller) is called without
the includeTimestamps option; update the return in the register handler to call
sellerToPublicJson(seller, { includeTimestamps: true }) so the response includes
created_at and updated_at and matches the shape returned by GET
/api/sellers/[pubkey] and POST /api/sellers/profile; ensure any related typing
or consumers still accept the timestamps.
| <div className="flex items-start gap-3"> | ||
| <div | ||
| className="grid h-14 w-14 shrink-0 place-items-center overflow-hidden rounded-[var(--radius)] border border-[var(--line)] bg-[var(--panel-strong)] bg-cover bg-center text-xl font-black text-[var(--accent-dark)]" | ||
| style={pictureUrl ? { backgroundImage: `url("${pictureUrl}")` } : undefined} | ||
| aria-hidden | ||
| > | ||
| {pictureUrl ? null : sellerAvatarInitial(listing.seller)} | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 '\b(picture|profile_picture|profilePictureUrl)\b' --type=ts
ast-grep --pattern 'z.string().url($$$)'Repository: ATLBitLab/freeport
Length of output: 9199
🏁 Script executed:
cat -n lib/validation.ts | head -100Repository: ATLBitLab/freeport
Length of output: 3736
🏁 Script executed:
cat -n lib/seller-profile.ts | sed -n '30,60p'Repository: ATLBitLab/freeport
Length of output: 1220
🏁 Script executed:
cat -n app/listings/\[id\]/page.tsx | sed -n '1,80p'Repository: ATLBitLab/freeport
Length of output: 4417
🏁 Script executed:
cat -n lib/seller-profile.ts | sed -n '41,70p'Repository: ATLBitLab/freeport
Length of output: 1150
🏁 Script executed:
cat -n lib/seller-profile.ts | grep -A 5 "STRING_LIMITS"Repository: ATLBitLab/freeport
Length of output: 790
🏁 Script executed:
# Test if the attack URL passes the URL constructor validation
python3 << 'EOF'
try:
from urllib.parse import urlparse
attack_url = 'https://ok.example/a.png"), url(https://attacker.example/log?ip='
parsed = urlparse(attack_url)
print(f"URL parsed successfully: {parsed}")
print(f"Scheme: {parsed.scheme}")
print(f"Netloc: {parsed.netloc}")
print(f"Path: {parsed.path}")
except Exception as e:
print(f"Error: {e}")
EOFRepository: ATLBitLab/freeport
Length of output: 306
🏁 Script executed:
# Also check with Node's URL constructor behavior
node << 'EOF'
try {
const attackUrl = 'https://ok.example/a.png"), url(https://attacker.example/log?ip=';
const url = new URL(attackUrl);
console.log("URL parsed successfully");
console.log("href:", url.href);
console.log("pathname:", url.pathname);
} catch (e) {
console.log("Error:", e.message);
}
EOFRepository: ATLBitLab/freeport
Length of output: 218
CSS injection / SSRF-on-render risk via profilePictureUrl in inline style.
The upstream validator in lib/seller-profile.ts (via readUrl) uses new URL() to ensure the URL has an http(s) protocol, but returns the original unescaped string. This is insufficient for CSS context: a URL like https://ok.example/a.png"), url(https://attacker.example/log?ip= passes validation but breaks the CSS string when embedded as url("${pictureUrl}"), producing two separate url() calls. The second call is fetched by the browser on render, enabling SSRF/IP leaking.
Validation does not and cannot escape the CSS metacharacters (", ), whitespace). Render via <img>/next/image instead, where the URL goes through src attribute parsing rather than CSS.
♻️ Suggested safer rendering
- <div
- className="grid h-14 w-14 shrink-0 place-items-center overflow-hidden rounded-[var(--radius)] border border-[var(--line)] bg-[var(--panel-strong)] bg-cover bg-center text-xl font-black text-[var(--accent-dark)]"
- style={pictureUrl ? { backgroundImage: `url("${pictureUrl}")` } : undefined}
- aria-hidden
- >
- {pictureUrl ? null : sellerAvatarInitial(listing.seller)}
- </div>
+ <div className="grid h-14 w-14 shrink-0 place-items-center overflow-hidden rounded-[var(--radius)] border border-[var(--line)] bg-[var(--panel-strong)] text-xl font-black text-[var(--accent-dark)]" aria-hidden>
+ {pictureUrl ? (
+ // eslint-disable-next-line `@next/next/no-img-element`
+ <img src={pictureUrl} alt="" className="h-full w-full object-cover" />
+ ) : (
+ sellerAvatarInitial(listing.seller)
+ )}
+ </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/listings/`[id]/page.tsx around lines 65 - 72, The inline style using
pictureUrl in the backgroundImage of the div (where
sellerAvatarInitial(listing.seller) is used as fallback) allows CSS
injection/SSRF via crafted URLs; stop embedding the URL inside style and instead
render a standard image element (e.g., <img> or next/image) with
src={pictureUrl} and the same classes for sizing/rounding, keeping
sellerAvatarInitial(listing.seller) as the visual fallback when pictureUrl is
falsy; remove the style prop and backgroundImage usage entirely so the browser
handles the URL via src parsing rather than CSS string interpolation.
| {label === "Website" ? ( | ||
| <a | ||
| className="break-all text-sm font-bold underline" | ||
| href={value} | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| > | ||
| {value} | ||
| </a> |
There was a problem hiding this comment.
Confirm profileWebsite is restricted to http(s).
<a href={value}> will follow a javascript: URL and execute it on click — a stored XSS if upstream validation only checks z.string().url() (which accepts any scheme). Please verify the validator pins the protocol to http/https, or guard at render time.
🛡️ Render-time guard fallback
- {label === "Website" ? (
- <a
- className="break-all text-sm font-bold underline"
- href={value}
- target="_blank"
- rel="noreferrer"
- >
- {value}
- </a>
+ {label === "Website" && /^https?:\/\//i.test(value) ? (
+ <a
+ className="break-all text-sm font-bold underline"
+ href={value}
+ target="_blank"
+ rel="noreferrer"
+ >
+ {value}
+ </a>#!/bin/bash
rg -nP -C3 'profileWebsite|\bwebsite\b' --type=ts -g '!**/node_modules/**'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/listings/`[id]/page.tsx around lines 91 - 99, The Website link can be a
javascript: XSS vector; ensure the upstream validator for profileWebsite
restricts schemes to http/https (e.g., pin protocols in your Zod/schema), and
add a render-time guard in app/listings/[id]/page.tsx where label === "Website"
(using the value variable) to only render the <a> if value begins with "http://"
or "https://"; otherwise omit the anchor or render a safe plain text fallback.
Also consider normalizing or rejecting non-HTTP(S) values when saving via the
profileWebsite field.
| <div | ||
| className="grid h-9 w-9 shrink-0 place-items-center overflow-hidden rounded-[var(--radius)] border border-[var(--line)] bg-[var(--panel-strong)] bg-cover bg-center text-sm font-black text-[var(--accent-dark)]" | ||
| style={pictureUrl ? { backgroundImage: `url("${pictureUrl}")` } : undefined} | ||
| aria-hidden | ||
| > | ||
| {pictureUrl ? null : sellerAvatarInitial(listing.seller)} | ||
| </div> |
There was a problem hiding this comment.
Same backgroundImage injection risk as the listing detail page.
If profilePictureUrl slips through validation containing "/), an attacker can append a second url(...) that the browser fetches on every card render (visible across the whole listings index). Apply the same fix here — render via <img> or strictly validate the URL — see the major issue raised on app/listings/[id]/page.tsx for details.
♻️ Suggested change
- <div
- className="grid h-9 w-9 shrink-0 place-items-center overflow-hidden rounded-[var(--radius)] border border-[var(--line)] bg-[var(--panel-strong)] bg-cover bg-center text-sm font-black text-[var(--accent-dark)]"
- style={pictureUrl ? { backgroundImage: `url("${pictureUrl}")` } : undefined}
- aria-hidden
- >
- {pictureUrl ? null : sellerAvatarInitial(listing.seller)}
- </div>
+ <div className="grid h-9 w-9 shrink-0 place-items-center overflow-hidden rounded-[var(--radius)] border border-[var(--line)] bg-[var(--panel-strong)] text-sm font-black text-[var(--accent-dark)]" aria-hidden>
+ {pictureUrl ? (
+ // eslint-disable-next-line `@next/next/no-img-element`
+ <img src={pictureUrl} alt="" className="h-full w-full object-cover" />
+ ) : (
+ sellerAvatarInitial(listing.seller)
+ )}
+ </div>📝 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.
| <div | |
| className="grid h-9 w-9 shrink-0 place-items-center overflow-hidden rounded-[var(--radius)] border border-[var(--line)] bg-[var(--panel-strong)] bg-cover bg-center text-sm font-black text-[var(--accent-dark)]" | |
| style={pictureUrl ? { backgroundImage: `url("${pictureUrl}")` } : undefined} | |
| aria-hidden | |
| > | |
| {pictureUrl ? null : sellerAvatarInitial(listing.seller)} | |
| </div> | |
| <div className="grid h-9 w-9 shrink-0 place-items-center overflow-hidden rounded-[var(--radius)] border border-[var(--line)] bg-[var(--panel-strong)] text-sm font-black text-[var(--accent-dark)]" aria-hidden> | |
| {pictureUrl ? ( | |
| // eslint-disable-next-line `@next/next/no-img-element` | |
| <img src={pictureUrl} alt="" className="h-full w-full object-cover" /> | |
| ) : ( | |
| sellerAvatarInitial(listing.seller) | |
| )} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/listing-card.tsx` around lines 57 - 63, The inline style using
backgroundImage with pictureUrl in listing-card.tsx (the div that currently uses
style={pictureUrl ? { backgroundImage: `url("${pictureUrl}")` } : undefined}) is
vulnerable to URL injection; replace this pattern by rendering a proper <img>
element for the avatar when pictureUrl exists (fall back to
sellerAvatarInitial(listing.seller) when absent) or, if you must keep background
images, strictly validate/sanitize pictureUrl (e.g., parse with the URL
constructor, enforce http(s) scheme and a hostname allowlist) before assigning
it to the style; update the component that references pictureUrl and
sellerAvatarInitial accordingly.
| async upsertSellerProfileFromEvent(event: NostrEvent, profile: SellerProfileData) { | ||
| const seller = await this.upsertSeller({ pubkey: event.pubkey }); | ||
| const profileUpdated = shouldReplaceSellerProfile(seller, event); | ||
| const eventRecord = eventRecordFromEvent(event, null); | ||
| if (!profileUpdated) eventRecord.supersededByEventId = seller.profileEventId; | ||
|
|
||
| if (!this.store.events.some((record) => record.eventId === event.id)) { | ||
| this.store.events.push(eventRecord); | ||
| } | ||
|
|
||
| if (!profileUpdated) { | ||
| return { seller, profileUpdated }; | ||
| } | ||
|
|
||
| seller.profileName = profile.profileName; | ||
| seller.profileDisplayName = profile.profileDisplayName; | ||
| seller.profileAbout = profile.profileAbout; | ||
| seller.profilePictureUrl = profile.profilePictureUrl; | ||
| seller.profileWebsite = profile.profileWebsite; | ||
| seller.profileNip05 = profile.profileNip05; | ||
| seller.profileLud16 = profile.profileLud16; | ||
| seller.profileBot = profile.profileBot; | ||
| seller.profileMetadata = profile.profileMetadata; | ||
| seller.profileEventId = event.id; | ||
| seller.profileEventCreatedAt = event.created_at; | ||
| seller.updatedAt = new Date().toISOString(); | ||
| return { seller, profileUpdated }; | ||
| } |
There was a problem hiding this comment.
Memory repo never updates the prior profile event's supersededByEventId.
When a newer event arrives and replaces the current profile, the Supabase path correctly updates the prior profile event's superseded_by_event_id (lines 402–407), but the in-memory implementation here only sets supersededByEventId on incoming older events (line 149) and never patches the previously-stored current event. After:
- Post profile A (current;
supersededByEventId=null) - Post profile B newer than A (becomes current)
…event A in this.store.events still has supersededByEventId === null, diverging from the Supabase behavior and from any UI/script that relies on supersession to decide which historical events to display.
🛠️ Proposed fix
seller.profileMetadata = profile.profileMetadata;
+ const previousProfileEventId = seller.profileEventId;
seller.profileEventId = event.id;
seller.profileEventCreatedAt = event.created_at;
seller.updatedAt = new Date().toISOString();
+ if (previousProfileEventId) {
+ const prior = this.store.events.find((record) => record.eventId === previousProfileEventId);
+ if (prior) prior.supersededByEventId = event.id;
+ }
return { seller, profileUpdated };📝 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.
| async upsertSellerProfileFromEvent(event: NostrEvent, profile: SellerProfileData) { | |
| const seller = await this.upsertSeller({ pubkey: event.pubkey }); | |
| const profileUpdated = shouldReplaceSellerProfile(seller, event); | |
| const eventRecord = eventRecordFromEvent(event, null); | |
| if (!profileUpdated) eventRecord.supersededByEventId = seller.profileEventId; | |
| if (!this.store.events.some((record) => record.eventId === event.id)) { | |
| this.store.events.push(eventRecord); | |
| } | |
| if (!profileUpdated) { | |
| return { seller, profileUpdated }; | |
| } | |
| seller.profileName = profile.profileName; | |
| seller.profileDisplayName = profile.profileDisplayName; | |
| seller.profileAbout = profile.profileAbout; | |
| seller.profilePictureUrl = profile.profilePictureUrl; | |
| seller.profileWebsite = profile.profileWebsite; | |
| seller.profileNip05 = profile.profileNip05; | |
| seller.profileLud16 = profile.profileLud16; | |
| seller.profileBot = profile.profileBot; | |
| seller.profileMetadata = profile.profileMetadata; | |
| seller.profileEventId = event.id; | |
| seller.profileEventCreatedAt = event.created_at; | |
| seller.updatedAt = new Date().toISOString(); | |
| return { seller, profileUpdated }; | |
| } | |
| async upsertSellerProfileFromEvent(event: NostrEvent, profile: SellerProfileData) { | |
| const seller = await this.upsertSeller({ pubkey: event.pubkey }); | |
| const profileUpdated = shouldReplaceSellerProfile(seller, event); | |
| const eventRecord = eventRecordFromEvent(event, null); | |
| if (!profileUpdated) eventRecord.supersededByEventId = seller.profileEventId; | |
| if (!this.store.events.some((record) => record.eventId === event.id)) { | |
| this.store.events.push(eventRecord); | |
| } | |
| if (!profileUpdated) { | |
| return { seller, profileUpdated }; | |
| } | |
| seller.profileName = profile.profileName; | |
| seller.profileDisplayName = profile.profileDisplayName; | |
| seller.profileAbout = profile.profileAbout; | |
| seller.profilePictureUrl = profile.profilePictureUrl; | |
| seller.profileWebsite = profile.profileWebsite; | |
| seller.profileNip05 = profile.profileNip05; | |
| seller.profileLud16 = profile.profileLud16; | |
| seller.profileBot = profile.profileBot; | |
| seller.profileMetadata = profile.profileMetadata; | |
| const previousProfileEventId = seller.profileEventId; | |
| seller.profileEventId = event.id; | |
| seller.profileEventCreatedAt = event.created_at; | |
| seller.updatedAt = new Date().toISOString(); | |
| if (previousProfileEventId) { | |
| const prior = this.store.events.find((record) => record.eventId === previousProfileEventId); | |
| if (prior) prior.supersededByEventId = event.id; | |
| } | |
| return { seller, profileUpdated }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/repository.ts` around lines 145 - 172, In upsertSellerProfileFromEvent,
when a new profile replaces the current one you must patch the previously-stored
event's supersededByEventId in the in-memory store just like the Supabase path
does; after computing profileUpdated and before pushing/returning, locate the
existing event record in this.store.events whose eventId equals
seller.profileEventId (the prior current event) and set its supersededByEventId
= event.id (and if you already set eventRecord.supersededByEventId for the
incoming record, only do that when the incoming event is older), so that
seller.profileEventId/seller.profileEventCreatedAt are updated and the previous
event reflects it in memory.
| async upsertSellerProfileFromEvent(event: NostrEvent, profile: SellerProfileData) { | ||
| const client = this.requireClient(); | ||
| const seller = await this.upsertSeller({ pubkey: event.pubkey }); | ||
| const profileUpdated = shouldReplaceSellerProfile(seller, event); | ||
| const eventRecord = eventRecordFromEvent(event, null); | ||
| if (!profileUpdated) eventRecord.supersededByEventId = seller.profileEventId; | ||
|
|
||
| const { error: eventError } = await client | ||
| .from("listing_events") | ||
| .upsert(eventRecordToRow(eventRecord), { | ||
| onConflict: "event_id", | ||
| ignoreDuplicates: true, | ||
| }); | ||
| if (eventError) throw eventError; | ||
|
|
||
| if (!profileUpdated) { | ||
| return { seller, profileUpdated }; | ||
| } | ||
|
|
||
| const { data, error } = await client | ||
| .from("sellers") | ||
| .update({ | ||
| profile_name: profile.profileName, | ||
| profile_display_name: profile.profileDisplayName, | ||
| profile_about: profile.profileAbout, | ||
| profile_picture_url: profile.profilePictureUrl, | ||
| profile_website: profile.profileWebsite, | ||
| profile_nip05: profile.profileNip05, | ||
| profile_lud16: profile.profileLud16, | ||
| profile_bot: profile.profileBot, | ||
| profile_metadata: profile.profileMetadata, | ||
| profile_event_id: event.id, | ||
| profile_event_created_at: event.created_at, | ||
| }) | ||
| .eq("id", seller.id) | ||
| .select("*") | ||
| .single(); | ||
| if (error) throw error; | ||
|
|
||
| if (seller.profileEventId) { | ||
| await client | ||
| .from("listing_events") | ||
| .update({ superseded_by_event_id: event.id }) | ||
| .eq("event_id", seller.profileEventId); | ||
| } | ||
|
|
||
| return { seller: sellerFromRow(data), profileUpdated }; | ||
| } |
There was a problem hiding this comment.
TOCTOU race: concurrent profile updates can leave the sellers row pointing at a stale event.
shouldReplaceSellerProfile is evaluated against an in-memory seller snapshot (read at line 365), but the subsequent update at lines 382–399 is unconditional (.eq("id", seller.id)). If two profile events for the same pubkey land concurrently and event B (newer created_at) commits first, event A (older) may still pass the in-memory check against the pre-B snapshot and then overwrite B's columns — corrupting NIP-01 replaceable-event semantics. The superseded_by_event_id update at lines 402–407 has the same issue and can mark the wrong event as superseded.
Consider making the seller update conditional so the database enforces ordering, e.g.:
🛡️ Sketch — make the row update self-guarding
const { data, error } = await client
.from("sellers")
.update({ /* ...profile_* fields... */
profile_event_id: event.id,
profile_event_created_at: event.created_at,
})
.eq("id", seller.id)
- .select("*")
- .single();
+ .or(
+ `profile_event_created_at.is.null,profile_event_created_at.lt.${event.created_at},and(profile_event_created_at.eq.${event.created_at},profile_event_id.gt.${event.id})`,
+ )
+ .select("*")
+ .maybeSingle();
+ if (error) throw error;
+ if (!data) {
+ // Lost the race; re-read seller and treat as not-updated.
+ const fresh = await this.getSellerByPubkey(event.pubkey);
+ return { seller: fresh ?? seller, profileUpdated: false };
+ }A SQL function or a unique partial index on (pubkey) plus an explicit comparison in WHERE would also work. For the common single-writer case this isn't critical, but it is reachable from any client that retries or fans out profile posts.
A unique index on sellers.pubkey (already implied) plus a per-seller advisory lock (pg_advisory_xact_lock(hashtext(pubkey))) inside a stored procedure would also serialize profile updates cleanly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/repository.ts` around lines 363 - 410, upsertSellerProfileFromEvent has a
TOCTOU: shouldReplaceSellerProfile is decided from the in-memory seller snapshot
but the .update on the "sellers" row is unconditional, so a
concurrently-committed newer profile can be overwritten; change the update to be
conditional in SQL (e.g., add a .eq("id",
seller.id).lt("profile_event_created_at", event.created_at) or compare
profile_event_id/profile_event_created_at in the WHERE) so the DB only accepts
the update if the stored profile is older than the incoming event; do the same
conditional check before updating listing_events.superseded_by_event_id (only
mark the previous event as superseded if you successfully updated the seller
row), and optionally guard or serialize using pg_advisory_xact_lock or a DB-side
stored procedure/unique constraint if you prefer DB-enforced ordering.
1 participant
Summary
Adds local ingestion for seller-signed standard Nostr
kind: 0profile metadata events. The new/api/sellers/profileendpoint verifies event ids and Schnorr signatures, validates rendered profile fields, preserves the full metadata object, stores every valid profile event inlisting_events, and applies NIP-01 replaceable-event ordering when deciding which profile updates the seller row.Also extends seller storage, public JSON serialization, listing/seller responses, listing card/detail rendering, docs, examples, and the agent helper script with profile signing/posting flows.
User Impact
Seller display data can now come from a signed profile note instead of only unsigned registration fields. Listing cards and detail pages prefer signed
display_name/name, render profile pictures from URL metadata, and show optional about, website, NIP-05, Lightning address, and bot fields.Validation
pnpm freeport:profile-checkpnpm lintpnpm buildfrom/tmp/freeport-build-checkafter copying the checkout to a hash-free pathnext devon port 3100 from the same/tmpcopy: generated a seller key, postedexamples/profile.json, then fetched/api/sellers/:pubkeyand confirmed signed profile fieldsNote: running
pnpm builddirectly in this worktree fails before app compilation because Turbopack/Tailwind mishandles the#9-nostr-profilesdirectory name and produces a null-byte path. The same code builds successfully from/tmp/freeport-build-check.Summary by CodeRabbit
Release Notes
New Features
/api/sellers/profileendpoint accepts signed seller profile submissionsDocumentation