Fix four public-data and consent leaks; add repo audit and agent skills - #119
Merged
Merged
Conversation
The unauthenticated community-data feed attributed each shared yield row to COALESCE(display_name, username). Firebase users' username is their verified email, so sharing a row without a contributor profile published the sharer's email, and the show_on_page consent flag was ignored. - Move query + row shaping into the shared handler core (handleListCommunityData) with a new DbAdapter method listSharedYieldRows, implemented for Neon and SQLite. Neither query joins users, so no account identifier is ever selected. - Attribution only with explicit consent: display name and organization appear only when a contributor profile exists and show_on_page is true (Postgres boolean or SQLite 1); otherwise anonymous. - Output rows are allowlisted (id, species, product, yield, source, contributor, organization); yield normalized to a number on both backends. CSV keeps formula-injection sanitization. - Express /api/community-data now honors ?format=csv like production; /api/export-community-data routes through the same handler. - Sharing a yield row now opens ShareYieldModal listing exactly what becomes public and how it will be attributed; cancel sends no request, unshare stays one click. - Tests: attribution cases, identifier-free JSON/CSV, SQL contents, in-memory SQLite parity, and the share/cancel/confirm flow. Closes #26 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dn84c88DDz5xSC19Z9c5Mg
The branch was rebuilt on current main; its old tip carried an agent-skills config commit that main now supersedes with its own. Recorded here as a parent so the push is a fast-forward with no history rewritten. Tree is unchanged from the tested fix commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dn84c88DDz5xSC19Z9c5Mg
The public contributors endpoint selected c.* plus u.username, so every row carried the account username (a verified email for Firebase users), the internal user_id and timestamps, even though the page renders only five fields. - Neon and SQLite listContributors now select only id, display_name, organization, bio and a contribution count. No join to users and no c.*, so a later edit cannot leak identifiers by accident. - New allowlist shaper (shared/handlers/contributorsList.js) applied in handleListContributors; contribution_count is normalized to a number so both backends emit the same type. - Profiles without a usable display name are not listed (the page would render an empty heading). Ordering gets an id tie-break for stability. - Tests: planted-identifier fake adapter, exact key set, Neon SQL text (no users table, no c.*), and in-memory SQLite exclusion/count/blank name cases. Closes #115 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dn84c88DDz5xSC19Z9c5Mg
Accounts created through Firebase sign-in have a NULL password. handleLogin passed that to bcrypt.compare, which threw and returned 500, while a nonexistent username returned 401. Anyone could therefore test whether an email address had an account, and a Firebase user who mistyped their password hit a server error via the login form's legacy fallback. - A missing user, an account with no usable password, and a wrong password all return the same 401 "Invalid credentials". - A bcrypt compare always runs, against a fixed valid dummy hash (cost 10, matching register) when there is no real one, so timing does not reveal which case occurred. A stored value that is not a well-formed bcrypt hash counts as no usable hash, since compare() would otherwise return instantly. Measured medians for all four cases are within ~2ms. - Database errors still return 500; success, 400 validation and the misconfiguration guard are unchanged. - Tests: null/empty/non-string/malformed stored passwords, identical responses, dummy compare on the not-found and no-hash paths. Registration's 409 on a taken username is the same signal on the sign-up side and is left to the decision in #27. Closes #117 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dn84c88DDz5xSC19Z9c5Mg
The contributor profile form loaded its consent checkbox with show_on_page === 1, which is only true for SQLite. Postgres returns a real boolean, so an opted-in production user saw the box unchecked, and saving any edit (e.g. a bio typo) withdrew their consent and removed them from the contributors listing and community attribution. - New app/src/lib/contributorProfile.js: normalizeShowOnPage (true, 1, 'true' -> checked; anything else unchecked), strict isExplicitOptIn, profileToFormData, buildProfilePayload. The component uses them for initial state, load and the POST body; no inline comparison remains. - The share preview's attribution check reuses the strict isExplicitOptIn so the preview never promises attribution the server withholds (the server does not treat the string 'true' as consent). - Review found the mirror-image risk: if the profile GET failed or had not finished, the form kept its opted-in default and a user with stored consent off could save and silently grant it. The form and submit are now disabled until the load resolves with 200 or 404, and handleSubmit refuses to run before then. - Tests: mapping table for boolean/int/string/garbage values and load-then-save round trips (true, 1, 'true', default). Closes #116 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dn84c88DDz5xSC19Z9c5Mg
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (63)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b421796981
ℹ️ 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".
…rs docs Addresses review feedback on #119. - Sharing a yield row is now refused unless the preview resolved how the row will be credited. If the contributor-profile request failed, the server would still apply the stored show_on_page consent and could publish a name the confirmation never displayed. The flow controller (the only path that calls the share API) returns an error and makes no request while attribution is loading or unavailable, and the dialog disables Share and says why. A user with no profile (404) still shares, anonymously. - docs/API.md: the GET /api/contributors example now shows the allowlisted response (no user_id, username or show_on_page). - Tests for both blocked states and the no-profile path. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dn84c88DDz5xSC19Z9c5Mg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a0bfa003f
ℹ️ 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".
…rofile) Addresses the second round of review feedback on #119. - Focus trap: disabled buttons/inputs no longer count as the first or last focusable element, so Tab wraps inside the dialog while Share is disabled (loading, unavailable, or sending) instead of escaping it. - A failed share (non-2xx or network error) is kept on the pending state and shown inside the open dialog with role="alert". It previously went to a page-level banner rendered behind the modal, leaving the user in the dialog with no explanation. The page banner is used only when the dialog has closed. - A late profile response is now ignored unless it belongs to the exact preview that is still open. Comparing only the row let an old request mark a reopened preview "ready" with stale attribution. Verified by a test that fails against the old guard. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dn84c88DDz5xSC19Z9c5Mg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42ae823396
ℹ️ 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".
…review The share API takes only the entry id and publishes the server copy, but the dialog previews the local fields. For a row with an unsynced local edit (or a conflict / pending delete) those differ: the dialog would show one set of fields while the older server values went public, and sharing bumps the server revision, so the pending edit would then conflict and the stale values could stay public. - getShareBlocker(): a row can be previewed and shared only when it exists on the server and its local syncStatus is 'synced'. - request() refuses before opening any preview (no profile request, no dialog), and confirm() re-checks the previewed snapshot, so a row that stops being synced is never published. The user is told to sync first. - Unsharing is unaffected: taking something private again is always allowed, including for rows with unsynced edits. - Tests: every non-synced state is refused with no preview and no API call, the confirm-time re-check, and unshare still works. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dn84c88DDz5xSC19Z9c5Mg
Owner
Author
|
@codex yeah. Please review. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
paccloud
pushed a commit
that referenced
this pull request
Sep 29, 2026
main (#119) already stops public endpoints exposing emails and user ids, with consent-aware attribution. Keep main's version of those files and narrow this PR to the owner-id-from-token fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnPh61Aps5neY87hh1pu9T
This was referenced Sep 29, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes four privacy and consent bugs found by re-triaging the closed audit issues against current
main, plus the audit report and the Matt Pocock agent skills.Fixes
c.*andu.username. It now selects only the five fields the page renders and no longer touches the users table.show_on_page === 1, which is false for Postgres booleans, so saving any edit turned consent off. The form now handles every stored representation, and it is disabled until the profile has loaded, so a failed load can no longer grant consent either.Also included
AUDIT_REPORT.md, the June repository audit. It predates a lot of work onmain, so treat it as a historical record..claude/skills/(40 files) that the repo'sCLAUDE.mdanddocs/agents/config already point at. Say so if you would rather install these locally and I will drop that commit.Review process
Each fix was implemented by one agent and then checked by two independent reviewers, one hunting for remaining leaks and one checking acceptance criteria and mutation-testing the new tests. Review findings were fixed before commit, including the consent race in #116 and a malformed-hash timing gap in #117.
Test plan
npm testinapp/: 2,263 passing (baseline onmainwas 2,197)npm run lint: 0 errors (2 pre-existing warnings)npm run build: passes/api/community-dataand/api/contributorsresponses contain nousername,emailoruser_idon the deployed preview.Not fixed here
Needs an owner decision, all open on the tracker:
Closes #26, closes #115, closes #116, closes #117
🤖 Generated with Claude Code
https://claude.ai/code/session_01Dn84c88DDz5xSC19Z9c5Mg
Generated by Claude Code