feat(frontend): upload expenditure receipts for real by nourshoreibah · Pull Request #307 · Code-4-Community/branch

nourshoreibah · 2026-08-04T00:28:27Z

ℹ️ Issue

The last of the frontend's simulated data. Stacked on #306 — base that branch, not main.

📝 Description

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, under a standing TODO: update to use real progress of uploaded file. Nothing was ever transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL — despite the form refusing to submit without a receipt.

Submitting the form now:

Requests a presigned PUT from GET /expenditures/upload-url (added in feat(expenditures): presigned S3 uploads for expenditure receipts #306) for the chosen project and the dropped filename.
Sends the file straight to S3 with real byte progress.
Passes the returned objectUrl to POST /expenditures as receiptUrl.

If the upload fails, no expenditure is recorded and the error is surfaced — previously a failure was impossible because there was no upload.

Why the upload moved to submit rather than staying on drop: the presigned URL is per project, and the project may not be chosen when the file lands; and a file dropped into a form the user then abandons would leave an orphaned object in the bucket. The user still sees real progress, now while saving. FileUpload becomes controlled — it reports the dropped file immediately (so FilePreview still appears at once) and renders whatever progress its parent passes down.

src/lib/upload.ts — putWithProgress uses XMLHttpRequest because fetch exposes no upload progress. Two details worth a look:

It sends the content type the URL was signed with, not file.type. S3 checks Content-Type against the signature, so a mismatch is a 403.
It sends no Authorization header. The signature is in the URL, and an extra header would not be part of what was signed.

✔️ Verification

In apps/frontend, all green:

npm run typecheck — clean
npm run lint — clean
npx jest — 260 passed, 2 skipped, 25 suites
npm run build — static export succeeds

Coverage:

New test/lib/upload.test.ts (7) drives a stand-in XHR: the PUT's method/URL/content type, the absence of an auth header, progress reporting, ignoring non-computable progress events, and rejection on non-2xx (carrying the status), network error, and abort.
AddExpenseModal.test.tsx — asserts the upload URL is requested for the right project and filename, that receiptUrl reaches the POST body, that the PUT uses application/pdf, that the upload strictly precedes the POST, and that a failed upload records nothing and calls no onSuccess.
FileUpload.test.tsx — replaces the two fake-timer tests for the simulated upload with the controlled-progress behaviour: onChange fires immediately on drop, no progress bar appears on drop, real transferred bytes render as a percentage, and clearing progress returns to the preview.

🏕️ (Optional) Future Work / Notes

Merge feat(expenditures): presigned S3 uploads for expenditure receipts #306 first, or GET /expenditures/upload-url 404s.
The existing expenditures list has no receipt column, so uploaded receipts are not yet viewable anywhere — worth a follow-up now that receipt_url will actually be populated.

🤖 Generated with Claude Code

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, with a standing TODO to use real progress. Nothing was transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL despite the form refusing to submit without a receipt. Submitting the form now requests a presigned PUT from GET /expenditures/upload-url, sends the file to S3 with real byte progress, and passes the returned object URL to POST /expenditures as receiptUrl. If the upload fails, no expenditure is recorded and the error is shown. The upload runs on submit rather than on drop because the presigned URL is per project -- the project may not be chosen when the file lands -- and because a file dropped into an abandoned form would otherwise orphan an object in the bucket. FileUpload is now controlled: it reports the dropped file immediately and renders whatever progress its parent passes down. putWithProgress uses XHR because fetch exposes no upload progress, and sends the content type the URL was signed with rather than file.type, which S3 checks against the signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

nourshoreibah · 2026-08-04T00:30:09Z

⚠️No CI will run on this PR while it is stacked. The workflows trigger on pull_request: branches: [main, develop], which matches the base branch — so a PR based on a feature branch gets no checks at all, including the frontend-ci / lambda-tests / terraform-plan-summary that branch protection requires.

GitHub retargets this to main automatically once the parent merges and its branch is deleted; CI runs from that point. Until then the verification in the description is local only (typecheck, lint, full jest suite, production build — all green).

* fix: address audit findings across lambdas, frontend and infra Fixes the actionable findings from a full-repo bug scan. Findings already covered by open PRs (#301-#307) are deliberately untouched. Security / data exposure: - Lock down the generated-reports S3 bucket. All four block_public_* were false and a bucket policy granted s3:GetObject to Principal "*", so reports (member emails, donor contacts, expenditure amounts) were world-readable at predictable keys. Reports are now served only through a presigned GET. - Grant the lambda role s3:PutObject/s3:GetObject on the reports bucket. It had no S3 permissions at all, so POST /reports/generate was failing AccessDenied; GetObject is additionally required because a presigned URL carries the signer's permissions. - Validate objectUrl on POST /reports against the bucket's own host, so an arbitrary (e.g. javascript:) URL can no longer be stored and rendered. - Sanitize fileName before interpolating it into an S3 key in GET /reports/upload-url. - Stop logging every user row (emails, admin flags) to CloudWatch. Correctness: - Lowercase emails in UserValidationUtils.validateEmail. POST /users stored them verbatim while POST /auth/register looks up email.toLowerCase(), so any invite containing uppercase was permanently unclaimable (403 INVITATION_REQUIRED). - POST /auth/register now checks numUpdatedRows on the invitation claim. A no-op claim previously still returned 201, leaving a Cognito user whose sub referenced no row, which broke every later login. Same check on the auto-link path. - DELETE /users/{userId} deletes the Cognito user too, so the address can be re-invited. - PATCH /users/{userId} rejects email changes. Email is the Cognito username and nothing synced it, so a change silently broke sign-in and reset. - POST /donations returns 404 for a missing donor/project instead of 500, and accepts numeric strings for amount and ids. - GET /projects/{id}/donors selects explicit columns; selectAll() over a 3-table join collided project_id and leaked the whole project row. - Reject non-numeric path ids on the users and projects {id} routes, which reached Postgres as NaN and surfaced as 500s. Features that were half-built: - Add GET /reports/{id}/download returning a presigned URL. Reports could be generated but never retrieved; the frontend used object_url only for a format label. - Wire up bulk delete on the reports page. It was stubbed behind a stale comment claiming no DELETE endpoint existed, though DELETE /reports/{id} has been there all along. - Switch the reports page to server-side pagination and surface transient delete/download failures in a non-blocking banner. - Pass report_type through POST /reports/generate and return file_type. Cleanup: - Single canonical region-qualified S3 URL helper; the two call sites disagreed and the region-less form only resolved via a redirect. - Drop the unused DonationValidationUtils import. - Reconcile the duplicated auth DTOs (isAdmin is now required in both). Full dedup needs a packaging change, since neither shared package can resolve the other without a new cross-dependency. - Gitignore lambda.zip. Users-lambda tests now mock the Cognito SDK: CI injects a real user pool id, and the DELETE tests target seeded ashley@branch.org, so they would otherwise have issued live AdminDeleteUser calls against production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: auto-format terraform and update documentation - Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com> * chore: regenerate lambda READMEs * refactor(types): make @branch/types the single source of the auth DTOs The auth DTOs were declared twice, in shared/types/auth-types.d.ts and shared/lambda-auth/src/types.ts, and had already drifted (isAdmin was optional in one and required in the other). The previous commit only reconciled the two copies and left a "keep these in sync" comment, which is just documented duplication. shared/lambda-auth now takes a file: dependency on @branch/types and re-exports the DTOs from it, so there is exactly one declaration. @branch/types stays a dependency-free leaf, which is what keeps the edge acyclic; lambdas are unaffected because they already depend on both packages, and the types are erased at compile time so nothing reaches the bundle. Adding a dependency to shared/lambda-auth changes the resolved tree for every lambda, so all six package-lock.json files are regenerated. They were already stale: none recorded lambda-auth's devDependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(reports): bind report object keys to their project, and review fixes Addresses the four review comments on #310. The significant one: POST /reports only checked that objectUrl pointed at the reports bucket, not that the key belonged to the project being written. A caller with access to project A could register a row with project_id A and an objectUrl under reports/B/, then read project B's report back through GET /reports/{id}/download, which authorizes off report.project_id. That defeats the access control this PR set out to add. Keys are now bound to their project via a single reportKeyPrefix() helper used for both construction and validation: POST /reports rejects a key outside the project's prefix, and the download route re-checks the stored key against report.project_id before presigning. The prefix check runs after the access check so 403/404 still take precedence. All existing rows match the prefix, since both key-construction paths already used it. Also: - Clear the reports-page selection on page change. With server-side pagination selectedIds could retain rows from a previous page and bulk delete would remove them unseen. - Skip the Cognito delete when COGNITO_USER_POOL_ID is unset instead of making a call that cannot succeed, and log it as a configuration error. cognitoDeleted already reported false in this case via the InvalidParameterException path. - Re-indent the POST /donations membership and try/catch blocks, whose closing braces read as though they closed the route. One regression test covers the cross-project key rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

nourshoreibah · 2026-08-04T00:28:27Z

ℹ️ Issue

The last of the frontend's simulated data. Stacked on #306 — base that branch, not main.

📝 Description

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, under a standing TODO: update to use real progress of uploaded file. Nothing was ever transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL — despite the form refusing to submit without a receipt.

Submitting the form now:

Requests a presigned PUT from GET /expenditures/upload-url (added in feat(expenditures): presigned S3 uploads for expenditure receipts #306) for the chosen project and the dropped filename.
Sends the file straight to S3 with real byte progress.
Passes the returned objectUrl to POST /expenditures as receiptUrl.

If the upload fails, no expenditure is recorded and the error is surfaced — previously a failure was impossible because there was no upload.

Why the upload moved to submit rather than staying on drop: the presigned URL is per project, and the project may not be chosen when the file lands; and a file dropped into a form the user then abandons would leave an orphaned object in the bucket. The user still sees real progress, now while saving. FileUpload becomes controlled — it reports the dropped file immediately (so FilePreview still appears at once) and renders whatever progress its parent passes down.

src/lib/upload.ts — putWithProgress uses XMLHttpRequest because fetch exposes no upload progress. Two details worth a look:

It sends the content type the URL was signed with, not file.type. S3 checks Content-Type against the signature, so a mismatch is a 403.
It sends no Authorization header. The signature is in the URL, and an extra header would not be part of what was signed.

✔️ Verification

In apps/frontend, all green:

npm run typecheck — clean
npm run lint — clean
npx jest — 260 passed, 2 skipped, 25 suites
npm run build — static export succeeds

Coverage:

New test/lib/upload.test.ts (7) drives a stand-in XHR: the PUT's method/URL/content type, the absence of an auth header, progress reporting, ignoring non-computable progress events, and rejection on non-2xx (carrying the status), network error, and abort.
AddExpenseModal.test.tsx — asserts the upload URL is requested for the right project and filename, that receiptUrl reaches the POST body, that the PUT uses application/pdf, that the upload strictly precedes the POST, and that a failed upload records nothing and calls no onSuccess.
FileUpload.test.tsx — replaces the two fake-timer tests for the simulated upload with the controlled-progress behaviour: onChange fires immediately on drop, no progress bar appears on drop, real transferred bytes render as a percentage, and clearing progress returns to the preview.

🏕️ (Optional) Future Work / Notes

Merge feat(expenditures): presigned S3 uploads for expenditure receipts #306 first, or GET /expenditures/upload-url 404s.
The existing expenditures list has no receipt column, so uploaded receipts are not yet viewable anywhere — worth a follow-up now that receipt_url will actually be populated.

🤖 Generated with Claude Code

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, with a standing TODO to use real progress. Nothing was transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL despite the form refusing to submit without a receipt. Submitting the form now requests a presigned PUT from GET /expenditures/upload-url, sends the file to S3 with real byte progress, and passes the returned object URL to POST /expenditures as receiptUrl. If the upload fails, no expenditure is recorded and the error is shown. The upload runs on submit rather than on drop because the presigned URL is per project -- the project may not be chosen when the file lands -- and because a file dropped into an abandoned form would otherwise orphan an object in the bucket. FileUpload is now controlled: it reports the dropped file immediately and renders whatever progress its parent passes down. putWithProgress uses XHR because fetch exposes no upload progress, and sends the content type the URL was signed with rather than file.type, which S3 checks against the signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

nourshoreibah · 2026-08-04T00:30:09Z

⚠️No CI will run on this PR while it is stacked. The workflows trigger on pull_request: branches: [main, develop], which matches the base branch — so a PR based on a feature branch gets no checks at all, including the frontend-ci / lambda-tests / terraform-plan-summary that branch protection requires.

GitHub retargets this to main automatically once the parent merges and its branch is deleted; CI runs from that point. Until then the verification in the description is local only (typecheck, lint, full jest suite, production build — all green).

* fix: address audit findings across lambdas, frontend and infra Fixes the actionable findings from a full-repo bug scan. Findings already covered by open PRs (#301-#307) are deliberately untouched. Security / data exposure: - Lock down the generated-reports S3 bucket. All four block_public_* were false and a bucket policy granted s3:GetObject to Principal "*", so reports (member emails, donor contacts, expenditure amounts) were world-readable at predictable keys. Reports are now served only through a presigned GET. - Grant the lambda role s3:PutObject/s3:GetObject on the reports bucket. It had no S3 permissions at all, so POST /reports/generate was failing AccessDenied; GetObject is additionally required because a presigned URL carries the signer's permissions. - Validate objectUrl on POST /reports against the bucket's own host, so an arbitrary (e.g. javascript:) URL can no longer be stored and rendered. - Sanitize fileName before interpolating it into an S3 key in GET /reports/upload-url. - Stop logging every user row (emails, admin flags) to CloudWatch. Correctness: - Lowercase emails in UserValidationUtils.validateEmail. POST /users stored them verbatim while POST /auth/register looks up email.toLowerCase(), so any invite containing uppercase was permanently unclaimable (403 INVITATION_REQUIRED). - POST /auth/register now checks numUpdatedRows on the invitation claim. A no-op claim previously still returned 201, leaving a Cognito user whose sub referenced no row, which broke every later login. Same check on the auto-link path. - DELETE /users/{userId} deletes the Cognito user too, so the address can be re-invited. - PATCH /users/{userId} rejects email changes. Email is the Cognito username and nothing synced it, so a change silently broke sign-in and reset. - POST /donations returns 404 for a missing donor/project instead of 500, and accepts numeric strings for amount and ids. - GET /projects/{id}/donors selects explicit columns; selectAll() over a 3-table join collided project_id and leaked the whole project row. - Reject non-numeric path ids on the users and projects {id} routes, which reached Postgres as NaN and surfaced as 500s. Features that were half-built: - Add GET /reports/{id}/download returning a presigned URL. Reports could be generated but never retrieved; the frontend used object_url only for a format label. - Wire up bulk delete on the reports page. It was stubbed behind a stale comment claiming no DELETE endpoint existed, though DELETE /reports/{id} has been there all along. - Switch the reports page to server-side pagination and surface transient delete/download failures in a non-blocking banner. - Pass report_type through POST /reports/generate and return file_type. Cleanup: - Single canonical region-qualified S3 URL helper; the two call sites disagreed and the region-less form only resolved via a redirect. - Drop the unused DonationValidationUtils import. - Reconcile the duplicated auth DTOs (isAdmin is now required in both). Full dedup needs a packaging change, since neither shared package can resolve the other without a new cross-dependency. - Gitignore lambda.zip. Users-lambda tests now mock the Cognito SDK: CI injects a real user pool id, and the DELETE tests target seeded ashley@branch.org, so they would otherwise have issued live AdminDeleteUser calls against production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: auto-format terraform and update documentation - Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com> * chore: regenerate lambda READMEs * refactor(types): make @branch/types the single source of the auth DTOs The auth DTOs were declared twice, in shared/types/auth-types.d.ts and shared/lambda-auth/src/types.ts, and had already drifted (isAdmin was optional in one and required in the other). The previous commit only reconciled the two copies and left a "keep these in sync" comment, which is just documented duplication. shared/lambda-auth now takes a file: dependency on @branch/types and re-exports the DTOs from it, so there is exactly one declaration. @branch/types stays a dependency-free leaf, which is what keeps the edge acyclic; lambdas are unaffected because they already depend on both packages, and the types are erased at compile time so nothing reaches the bundle. Adding a dependency to shared/lambda-auth changes the resolved tree for every lambda, so all six package-lock.json files are regenerated. They were already stale: none recorded lambda-auth's devDependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(reports): bind report object keys to their project, and review fixes Addresses the four review comments on #310. The significant one: POST /reports only checked that objectUrl pointed at the reports bucket, not that the key belonged to the project being written. A caller with access to project A could register a row with project_id A and an objectUrl under reports/B/, then read project B's report back through GET /reports/{id}/download, which authorizes off report.project_id. That defeats the access control this PR set out to add. Keys are now bound to their project via a single reportKeyPrefix() helper used for both construction and validation: POST /reports rejects a key outside the project's prefix, and the download route re-checks the stored key against report.project_id before presigning. The prefix check runs after the access check so 403/404 still take precedence. All existing rows match the prefix, since both key-construction paths already used it. Also: - Clear the reports-page selection on page change. With server-side pagination selectedIds could retain rows from a previous page and bulk delete would remove them unseen. - Skip the Cognito delete when COGNITO_USER_POOL_ID is unset instead of making a call that cannot succeed, and log it as a configuration error. cognitoDeleted already reported false in this case via the InvalidParameterException path. - Re-indent the POST /donations membership and try/catch blocks, whose closing braces read as though they closed the route. One regression test covers the cross-project key rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

nourshoreibah · 2026-08-04T00:28:27Z

ℹ️ Issue

The last of the frontend's simulated data. Stacked on #306 — base that branch, not main.

📝 Description

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, under a standing TODO: update to use real progress of uploaded file. Nothing was ever transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL — despite the form refusing to submit without a receipt.

Submitting the form now:

Requests a presigned PUT from GET /expenditures/upload-url (added in feat(expenditures): presigned S3 uploads for expenditure receipts #306) for the chosen project and the dropped filename.
Sends the file straight to S3 with real byte progress.
Passes the returned objectUrl to POST /expenditures as receiptUrl.

If the upload fails, no expenditure is recorded and the error is surfaced — previously a failure was impossible because there was no upload.

Why the upload moved to submit rather than staying on drop: the presigned URL is per project, and the project may not be chosen when the file lands; and a file dropped into a form the user then abandons would leave an orphaned object in the bucket. The user still sees real progress, now while saving. FileUpload becomes controlled — it reports the dropped file immediately (so FilePreview still appears at once) and renders whatever progress its parent passes down.

src/lib/upload.ts — putWithProgress uses XMLHttpRequest because fetch exposes no upload progress. Two details worth a look:

It sends the content type the URL was signed with, not file.type. S3 checks Content-Type against the signature, so a mismatch is a 403.
It sends no Authorization header. The signature is in the URL, and an extra header would not be part of what was signed.

✔️ Verification

In apps/frontend, all green:

npm run typecheck — clean
npm run lint — clean
npx jest — 260 passed, 2 skipped, 25 suites
npm run build — static export succeeds

Coverage:

New test/lib/upload.test.ts (7) drives a stand-in XHR: the PUT's method/URL/content type, the absence of an auth header, progress reporting, ignoring non-computable progress events, and rejection on non-2xx (carrying the status), network error, and abort.
AddExpenseModal.test.tsx — asserts the upload URL is requested for the right project and filename, that receiptUrl reaches the POST body, that the PUT uses application/pdf, that the upload strictly precedes the POST, and that a failed upload records nothing and calls no onSuccess.
FileUpload.test.tsx — replaces the two fake-timer tests for the simulated upload with the controlled-progress behaviour: onChange fires immediately on drop, no progress bar appears on drop, real transferred bytes render as a percentage, and clearing progress returns to the preview.

🏕️ (Optional) Future Work / Notes

Merge feat(expenditures): presigned S3 uploads for expenditure receipts #306 first, or GET /expenditures/upload-url 404s.
The existing expenditures list has no receipt column, so uploaded receipts are not yet viewable anywhere — worth a follow-up now that receipt_url will actually be populated.

🤖 Generated with Claude Code

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, with a standing TODO to use real progress. Nothing was transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL despite the form refusing to submit without a receipt. Submitting the form now requests a presigned PUT from GET /expenditures/upload-url, sends the file to S3 with real byte progress, and passes the returned object URL to POST /expenditures as receiptUrl. If the upload fails, no expenditure is recorded and the error is shown. The upload runs on submit rather than on drop because the presigned URL is per project -- the project may not be chosen when the file lands -- and because a file dropped into an abandoned form would otherwise orphan an object in the bucket. FileUpload is now controlled: it reports the dropped file immediately and renders whatever progress its parent passes down. putWithProgress uses XHR because fetch exposes no upload progress, and sends the content type the URL was signed with rather than file.type, which S3 checks against the signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

nourshoreibah · 2026-08-04T00:30:09Z

⚠️No CI will run on this PR while it is stacked. The workflows trigger on pull_request: branches: [main, develop], which matches the base branch — so a PR based on a feature branch gets no checks at all, including the frontend-ci / lambda-tests / terraform-plan-summary that branch protection requires.

GitHub retargets this to main automatically once the parent merges and its branch is deleted; CI runs from that point. Until then the verification in the description is local only (typecheck, lint, full jest suite, production build — all green).

* fix: address audit findings across lambdas, frontend and infra Fixes the actionable findings from a full-repo bug scan. Findings already covered by open PRs (#301-#307) are deliberately untouched. Security / data exposure: - Lock down the generated-reports S3 bucket. All four block_public_* were false and a bucket policy granted s3:GetObject to Principal "*", so reports (member emails, donor contacts, expenditure amounts) were world-readable at predictable keys. Reports are now served only through a presigned GET. - Grant the lambda role s3:PutObject/s3:GetObject on the reports bucket. It had no S3 permissions at all, so POST /reports/generate was failing AccessDenied; GetObject is additionally required because a presigned URL carries the signer's permissions. - Validate objectUrl on POST /reports against the bucket's own host, so an arbitrary (e.g. javascript:) URL can no longer be stored and rendered. - Sanitize fileName before interpolating it into an S3 key in GET /reports/upload-url. - Stop logging every user row (emails, admin flags) to CloudWatch. Correctness: - Lowercase emails in UserValidationUtils.validateEmail. POST /users stored them verbatim while POST /auth/register looks up email.toLowerCase(), so any invite containing uppercase was permanently unclaimable (403 INVITATION_REQUIRED). - POST /auth/register now checks numUpdatedRows on the invitation claim. A no-op claim previously still returned 201, leaving a Cognito user whose sub referenced no row, which broke every later login. Same check on the auto-link path. - DELETE /users/{userId} deletes the Cognito user too, so the address can be re-invited. - PATCH /users/{userId} rejects email changes. Email is the Cognito username and nothing synced it, so a change silently broke sign-in and reset. - POST /donations returns 404 for a missing donor/project instead of 500, and accepts numeric strings for amount and ids. - GET /projects/{id}/donors selects explicit columns; selectAll() over a 3-table join collided project_id and leaked the whole project row. - Reject non-numeric path ids on the users and projects {id} routes, which reached Postgres as NaN and surfaced as 500s. Features that were half-built: - Add GET /reports/{id}/download returning a presigned URL. Reports could be generated but never retrieved; the frontend used object_url only for a format label. - Wire up bulk delete on the reports page. It was stubbed behind a stale comment claiming no DELETE endpoint existed, though DELETE /reports/{id} has been there all along. - Switch the reports page to server-side pagination and surface transient delete/download failures in a non-blocking banner. - Pass report_type through POST /reports/generate and return file_type. Cleanup: - Single canonical region-qualified S3 URL helper; the two call sites disagreed and the region-less form only resolved via a redirect. - Drop the unused DonationValidationUtils import. - Reconcile the duplicated auth DTOs (isAdmin is now required in both). Full dedup needs a packaging change, since neither shared package can resolve the other without a new cross-dependency. - Gitignore lambda.zip. Users-lambda tests now mock the Cognito SDK: CI injects a real user pool id, and the DELETE tests target seeded ashley@branch.org, so they would otherwise have issued live AdminDeleteUser calls against production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: auto-format terraform and update documentation - Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com> * chore: regenerate lambda READMEs * refactor(types): make @branch/types the single source of the auth DTOs The auth DTOs were declared twice, in shared/types/auth-types.d.ts and shared/lambda-auth/src/types.ts, and had already drifted (isAdmin was optional in one and required in the other). The previous commit only reconciled the two copies and left a "keep these in sync" comment, which is just documented duplication. shared/lambda-auth now takes a file: dependency on @branch/types and re-exports the DTOs from it, so there is exactly one declaration. @branch/types stays a dependency-free leaf, which is what keeps the edge acyclic; lambdas are unaffected because they already depend on both packages, and the types are erased at compile time so nothing reaches the bundle. Adding a dependency to shared/lambda-auth changes the resolved tree for every lambda, so all six package-lock.json files are regenerated. They were already stale: none recorded lambda-auth's devDependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(reports): bind report object keys to their project, and review fixes Addresses the four review comments on #310. The significant one: POST /reports only checked that objectUrl pointed at the reports bucket, not that the key belonged to the project being written. A caller with access to project A could register a row with project_id A and an objectUrl under reports/B/, then read project B's report back through GET /reports/{id}/download, which authorizes off report.project_id. That defeats the access control this PR set out to add. Keys are now bound to their project via a single reportKeyPrefix() helper used for both construction and validation: POST /reports rejects a key outside the project's prefix, and the download route re-checks the stored key against report.project_id before presigning. The prefix check runs after the access check so 403/404 still take precedence. All existing rows match the prefix, since both key-construction paths already used it. Also: - Clear the reports-page selection on page change. With server-side pagination selectedIds could retain rows from a previous page and bulk delete would remove them unseen. - Skip the Cognito delete when COGNITO_USER_POOL_ID is unset instead of making a call that cannot succeed, and log it as a configuration error. cognitoDeleted already reported false in this case via the InvalidParameterException path. - Re-indent the POST /donations membership and try/catch blocks, whose closing braces read as though they closed the route. One regression test covers the cross-project key rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

nourshoreibah · 2026-08-04T00:28:27Z

ℹ️ Issue

The last of the frontend's simulated data. Stacked on #306 — base that branch, not main.

📝 Description

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, under a standing TODO: update to use real progress of uploaded file. Nothing was ever transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL — despite the form refusing to submit without a receipt.

Submitting the form now:

Requests a presigned PUT from GET /expenditures/upload-url (added in feat(expenditures): presigned S3 uploads for expenditure receipts #306) for the chosen project and the dropped filename.
Sends the file straight to S3 with real byte progress.
Passes the returned objectUrl to POST /expenditures as receiptUrl.

If the upload fails, no expenditure is recorded and the error is surfaced — previously a failure was impossible because there was no upload.

Why the upload moved to submit rather than staying on drop: the presigned URL is per project, and the project may not be chosen when the file lands; and a file dropped into a form the user then abandons would leave an orphaned object in the bucket. The user still sees real progress, now while saving. FileUpload becomes controlled — it reports the dropped file immediately (so FilePreview still appears at once) and renders whatever progress its parent passes down.

src/lib/upload.ts — putWithProgress uses XMLHttpRequest because fetch exposes no upload progress. Two details worth a look:

It sends the content type the URL was signed with, not file.type. S3 checks Content-Type against the signature, so a mismatch is a 403.
It sends no Authorization header. The signature is in the URL, and an extra header would not be part of what was signed.

✔️ Verification

In apps/frontend, all green:

npm run typecheck — clean
npm run lint — clean
npx jest — 260 passed, 2 skipped, 25 suites
npm run build — static export succeeds

Coverage:

New test/lib/upload.test.ts (7) drives a stand-in XHR: the PUT's method/URL/content type, the absence of an auth header, progress reporting, ignoring non-computable progress events, and rejection on non-2xx (carrying the status), network error, and abort.
AddExpenseModal.test.tsx — asserts the upload URL is requested for the right project and filename, that receiptUrl reaches the POST body, that the PUT uses application/pdf, that the upload strictly precedes the POST, and that a failed upload records nothing and calls no onSuccess.
FileUpload.test.tsx — replaces the two fake-timer tests for the simulated upload with the controlled-progress behaviour: onChange fires immediately on drop, no progress bar appears on drop, real transferred bytes render as a percentage, and clearing progress returns to the preview.

🏕️ (Optional) Future Work / Notes

Merge feat(expenditures): presigned S3 uploads for expenditure receipts #306 first, or GET /expenditures/upload-url 404s.
The existing expenditures list has no receipt column, so uploaded receipts are not yet viewable anywhere — worth a follow-up now that receipt_url will actually be populated.

🤖 Generated with Claude Code

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, with a standing TODO to use real progress. Nothing was transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL despite the form refusing to submit without a receipt. Submitting the form now requests a presigned PUT from GET /expenditures/upload-url, sends the file to S3 with real byte progress, and passes the returned object URL to POST /expenditures as receiptUrl. If the upload fails, no expenditure is recorded and the error is shown. The upload runs on submit rather than on drop because the presigned URL is per project -- the project may not be chosen when the file lands -- and because a file dropped into an abandoned form would otherwise orphan an object in the bucket. FileUpload is now controlled: it reports the dropped file immediately and renders whatever progress its parent passes down. putWithProgress uses XHR because fetch exposes no upload progress, and sends the content type the URL was signed with rather than file.type, which S3 checks against the signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

nourshoreibah · 2026-08-04T00:30:09Z

⚠️No CI will run on this PR while it is stacked. The workflows trigger on pull_request: branches: [main, develop], which matches the base branch — so a PR based on a feature branch gets no checks at all, including the frontend-ci / lambda-tests / terraform-plan-summary that branch protection requires.

GitHub retargets this to main automatically once the parent merges and its branch is deleted; CI runs from that point. Until then the verification in the description is local only (typecheck, lint, full jest suite, production build — all green).

* fix: address audit findings across lambdas, frontend and infra Fixes the actionable findings from a full-repo bug scan. Findings already covered by open PRs (#301-#307) are deliberately untouched. Security / data exposure: - Lock down the generated-reports S3 bucket. All four block_public_* were false and a bucket policy granted s3:GetObject to Principal "*", so reports (member emails, donor contacts, expenditure amounts) were world-readable at predictable keys. Reports are now served only through a presigned GET. - Grant the lambda role s3:PutObject/s3:GetObject on the reports bucket. It had no S3 permissions at all, so POST /reports/generate was failing AccessDenied; GetObject is additionally required because a presigned URL carries the signer's permissions. - Validate objectUrl on POST /reports against the bucket's own host, so an arbitrary (e.g. javascript:) URL can no longer be stored and rendered. - Sanitize fileName before interpolating it into an S3 key in GET /reports/upload-url. - Stop logging every user row (emails, admin flags) to CloudWatch. Correctness: - Lowercase emails in UserValidationUtils.validateEmail. POST /users stored them verbatim while POST /auth/register looks up email.toLowerCase(), so any invite containing uppercase was permanently unclaimable (403 INVITATION_REQUIRED). - POST /auth/register now checks numUpdatedRows on the invitation claim. A no-op claim previously still returned 201, leaving a Cognito user whose sub referenced no row, which broke every later login. Same check on the auto-link path. - DELETE /users/{userId} deletes the Cognito user too, so the address can be re-invited. - PATCH /users/{userId} rejects email changes. Email is the Cognito username and nothing synced it, so a change silently broke sign-in and reset. - POST /donations returns 404 for a missing donor/project instead of 500, and accepts numeric strings for amount and ids. - GET /projects/{id}/donors selects explicit columns; selectAll() over a 3-table join collided project_id and leaked the whole project row. - Reject non-numeric path ids on the users and projects {id} routes, which reached Postgres as NaN and surfaced as 500s. Features that were half-built: - Add GET /reports/{id}/download returning a presigned URL. Reports could be generated but never retrieved; the frontend used object_url only for a format label. - Wire up bulk delete on the reports page. It was stubbed behind a stale comment claiming no DELETE endpoint existed, though DELETE /reports/{id} has been there all along. - Switch the reports page to server-side pagination and surface transient delete/download failures in a non-blocking banner. - Pass report_type through POST /reports/generate and return file_type. Cleanup: - Single canonical region-qualified S3 URL helper; the two call sites disagreed and the region-less form only resolved via a redirect. - Drop the unused DonationValidationUtils import. - Reconcile the duplicated auth DTOs (isAdmin is now required in both). Full dedup needs a packaging change, since neither shared package can resolve the other without a new cross-dependency. - Gitignore lambda.zip. Users-lambda tests now mock the Cognito SDK: CI injects a real user pool id, and the DELETE tests target seeded ashley@branch.org, so they would otherwise have issued live AdminDeleteUser calls against production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: auto-format terraform and update documentation - Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com> * chore: regenerate lambda READMEs * refactor(types): make @branch/types the single source of the auth DTOs The auth DTOs were declared twice, in shared/types/auth-types.d.ts and shared/lambda-auth/src/types.ts, and had already drifted (isAdmin was optional in one and required in the other). The previous commit only reconciled the two copies and left a "keep these in sync" comment, which is just documented duplication. shared/lambda-auth now takes a file: dependency on @branch/types and re-exports the DTOs from it, so there is exactly one declaration. @branch/types stays a dependency-free leaf, which is what keeps the edge acyclic; lambdas are unaffected because they already depend on both packages, and the types are erased at compile time so nothing reaches the bundle. Adding a dependency to shared/lambda-auth changes the resolved tree for every lambda, so all six package-lock.json files are regenerated. They were already stale: none recorded lambda-auth's devDependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(reports): bind report object keys to their project, and review fixes Addresses the four review comments on #310. The significant one: POST /reports only checked that objectUrl pointed at the reports bucket, not that the key belonged to the project being written. A caller with access to project A could register a row with project_id A and an objectUrl under reports/B/, then read project B's report back through GET /reports/{id}/download, which authorizes off report.project_id. That defeats the access control this PR set out to add. Keys are now bound to their project via a single reportKeyPrefix() helper used for both construction and validation: POST /reports rejects a key outside the project's prefix, and the download route re-checks the stored key against report.project_id before presigning. The prefix check runs after the access check so 403/404 still take precedence. All existing rows match the prefix, since both key-construction paths already used it. Also: - Clear the reports-page selection on page change. With server-side pagination selectedIds could retain rows from a previous page and bulk delete would remove them unseen. - Skip the Cognito delete when COGNITO_USER_POOL_ID is unset instead of making a call that cannot succeed, and log it as a configuration error. cognitoDeleted already reported false in this case via the InvalidParameterException path. - Re-indent the POST /donations membership and try/catch blocks, whose closing braces read as though they closed the route. One regression test covers the cross-project key rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

nourshoreibah · 2026-08-04T00:28:27Z

ℹ️ Issue

The last of the frontend's simulated data. Stacked on #306 — base that branch, not main.

📝 Description

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, under a standing TODO: update to use real progress of uploaded file. Nothing was ever transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL — despite the form refusing to submit without a receipt.

Submitting the form now:

Requests a presigned PUT from GET /expenditures/upload-url (added in feat(expenditures): presigned S3 uploads for expenditure receipts #306) for the chosen project and the dropped filename.
Sends the file straight to S3 with real byte progress.
Passes the returned objectUrl to POST /expenditures as receiptUrl.

If the upload fails, no expenditure is recorded and the error is surfaced — previously a failure was impossible because there was no upload.

Why the upload moved to submit rather than staying on drop: the presigned URL is per project, and the project may not be chosen when the file lands; and a file dropped into a form the user then abandons would leave an orphaned object in the bucket. The user still sees real progress, now while saving. FileUpload becomes controlled — it reports the dropped file immediately (so FilePreview still appears at once) and renders whatever progress its parent passes down.

src/lib/upload.ts — putWithProgress uses XMLHttpRequest because fetch exposes no upload progress. Two details worth a look:

It sends the content type the URL was signed with, not file.type. S3 checks Content-Type against the signature, so a mismatch is a 403.
It sends no Authorization header. The signature is in the URL, and an extra header would not be part of what was signed.

✔️ Verification

In apps/frontend, all green:

npm run typecheck — clean
npm run lint — clean
npx jest — 260 passed, 2 skipped, 25 suites
npm run build — static export succeeds

Coverage:

New test/lib/upload.test.ts (7) drives a stand-in XHR: the PUT's method/URL/content type, the absence of an auth header, progress reporting, ignoring non-computable progress events, and rejection on non-2xx (carrying the status), network error, and abort.
AddExpenseModal.test.tsx — asserts the upload URL is requested for the right project and filename, that receiptUrl reaches the POST body, that the PUT uses application/pdf, that the upload strictly precedes the POST, and that a failed upload records nothing and calls no onSuccess.
FileUpload.test.tsx — replaces the two fake-timer tests for the simulated upload with the controlled-progress behaviour: onChange fires immediately on drop, no progress bar appears on drop, real transferred bytes render as a percentage, and clearing progress returns to the preview.

🏕️ (Optional) Future Work / Notes

Merge feat(expenditures): presigned S3 uploads for expenditure receipts #306 first, or GET /expenditures/upload-url 404s.
The existing expenditures list has no receipt column, so uploaded receipts are not yet viewable anywhere — worth a follow-up now that receipt_url will actually be populated.

🤖 Generated with Claude Code

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, with a standing TODO to use real progress. Nothing was transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL despite the form refusing to submit without a receipt. Submitting the form now requests a presigned PUT from GET /expenditures/upload-url, sends the file to S3 with real byte progress, and passes the returned object URL to POST /expenditures as receiptUrl. If the upload fails, no expenditure is recorded and the error is shown. The upload runs on submit rather than on drop because the presigned URL is per project -- the project may not be chosen when the file lands -- and because a file dropped into an abandoned form would otherwise orphan an object in the bucket. FileUpload is now controlled: it reports the dropped file immediately and renders whatever progress its parent passes down. putWithProgress uses XHR because fetch exposes no upload progress, and sends the content type the URL was signed with rather than file.type, which S3 checks against the signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

nourshoreibah · 2026-08-04T00:30:09Z

⚠️No CI will run on this PR while it is stacked. The workflows trigger on pull_request: branches: [main, develop], which matches the base branch — so a PR based on a feature branch gets no checks at all, including the frontend-ci / lambda-tests / terraform-plan-summary that branch protection requires.

GitHub retargets this to main automatically once the parent merges and its branch is deleted; CI runs from that point. Until then the verification in the description is local only (typecheck, lint, full jest suite, production build — all green).

* fix: address audit findings across lambdas, frontend and infra Fixes the actionable findings from a full-repo bug scan. Findings already covered by open PRs (#301-#307) are deliberately untouched. Security / data exposure: - Lock down the generated-reports S3 bucket. All four block_public_* were false and a bucket policy granted s3:GetObject to Principal "*", so reports (member emails, donor contacts, expenditure amounts) were world-readable at predictable keys. Reports are now served only through a presigned GET. - Grant the lambda role s3:PutObject/s3:GetObject on the reports bucket. It had no S3 permissions at all, so POST /reports/generate was failing AccessDenied; GetObject is additionally required because a presigned URL carries the signer's permissions. - Validate objectUrl on POST /reports against the bucket's own host, so an arbitrary (e.g. javascript:) URL can no longer be stored and rendered. - Sanitize fileName before interpolating it into an S3 key in GET /reports/upload-url. - Stop logging every user row (emails, admin flags) to CloudWatch. Correctness: - Lowercase emails in UserValidationUtils.validateEmail. POST /users stored them verbatim while POST /auth/register looks up email.toLowerCase(), so any invite containing uppercase was permanently unclaimable (403 INVITATION_REQUIRED). - POST /auth/register now checks numUpdatedRows on the invitation claim. A no-op claim previously still returned 201, leaving a Cognito user whose sub referenced no row, which broke every later login. Same check on the auto-link path. - DELETE /users/{userId} deletes the Cognito user too, so the address can be re-invited. - PATCH /users/{userId} rejects email changes. Email is the Cognito username and nothing synced it, so a change silently broke sign-in and reset. - POST /donations returns 404 for a missing donor/project instead of 500, and accepts numeric strings for amount and ids. - GET /projects/{id}/donors selects explicit columns; selectAll() over a 3-table join collided project_id and leaked the whole project row. - Reject non-numeric path ids on the users and projects {id} routes, which reached Postgres as NaN and surfaced as 500s. Features that were half-built: - Add GET /reports/{id}/download returning a presigned URL. Reports could be generated but never retrieved; the frontend used object_url only for a format label. - Wire up bulk delete on the reports page. It was stubbed behind a stale comment claiming no DELETE endpoint existed, though DELETE /reports/{id} has been there all along. - Switch the reports page to server-side pagination and surface transient delete/download failures in a non-blocking banner. - Pass report_type through POST /reports/generate and return file_type. Cleanup: - Single canonical region-qualified S3 URL helper; the two call sites disagreed and the region-less form only resolved via a redirect. - Drop the unused DonationValidationUtils import. - Reconcile the duplicated auth DTOs (isAdmin is now required in both). Full dedup needs a packaging change, since neither shared package can resolve the other without a new cross-dependency. - Gitignore lambda.zip. Users-lambda tests now mock the Cognito SDK: CI injects a real user pool id, and the DELETE tests target seeded ashley@branch.org, so they would otherwise have issued live AdminDeleteUser calls against production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: auto-format terraform and update documentation - Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com> * chore: regenerate lambda READMEs * refactor(types): make @branch/types the single source of the auth DTOs The auth DTOs were declared twice, in shared/types/auth-types.d.ts and shared/lambda-auth/src/types.ts, and had already drifted (isAdmin was optional in one and required in the other). The previous commit only reconciled the two copies and left a "keep these in sync" comment, which is just documented duplication. shared/lambda-auth now takes a file: dependency on @branch/types and re-exports the DTOs from it, so there is exactly one declaration. @branch/types stays a dependency-free leaf, which is what keeps the edge acyclic; lambdas are unaffected because they already depend on both packages, and the types are erased at compile time so nothing reaches the bundle. Adding a dependency to shared/lambda-auth changes the resolved tree for every lambda, so all six package-lock.json files are regenerated. They were already stale: none recorded lambda-auth's devDependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(reports): bind report object keys to their project, and review fixes Addresses the four review comments on #310. The significant one: POST /reports only checked that objectUrl pointed at the reports bucket, not that the key belonged to the project being written. A caller with access to project A could register a row with project_id A and an objectUrl under reports/B/, then read project B's report back through GET /reports/{id}/download, which authorizes off report.project_id. That defeats the access control this PR set out to add. Keys are now bound to their project via a single reportKeyPrefix() helper used for both construction and validation: POST /reports rejects a key outside the project's prefix, and the download route re-checks the stored key against report.project_id before presigning. The prefix check runs after the access check so 403/404 still take precedence. All existing rows match the prefix, since both key-construction paths already used it. Also: - Clear the reports-page selection on page change. With server-side pagination selectedIds could retain rows from a previous page and bulk delete would remove them unseen. - Skip the Cognito delete when COGNITO_USER_POOL_ID is unset instead of making a call that cannot succeed, and log it as a configuration error. cognitoDeleted already reported false in this case via the InvalidParameterException path. - Re-indent the POST /donations membership and try/catch blocks, whose closing braces read as though they closed the route. One regression test covers the cross-project key rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

nourshoreibah · 2026-08-04T00:28:27Z

ℹ️ Issue

The last of the frontend's simulated data. Stacked on #306 — base that branch, not main.

📝 Description

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, under a standing TODO: update to use real progress of uploaded file. Nothing was ever transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL — despite the form refusing to submit without a receipt.

Submitting the form now:

Requests a presigned PUT from GET /expenditures/upload-url (added in feat(expenditures): presigned S3 uploads for expenditure receipts #306) for the chosen project and the dropped filename.
Sends the file straight to S3 with real byte progress.
Passes the returned objectUrl to POST /expenditures as receiptUrl.

If the upload fails, no expenditure is recorded and the error is surfaced — previously a failure was impossible because there was no upload.

Why the upload moved to submit rather than staying on drop: the presigned URL is per project, and the project may not be chosen when the file lands; and a file dropped into a form the user then abandons would leave an orphaned object in the bucket. The user still sees real progress, now while saving. FileUpload becomes controlled — it reports the dropped file immediately (so FilePreview still appears at once) and renders whatever progress its parent passes down.

src/lib/upload.ts — putWithProgress uses XMLHttpRequest because fetch exposes no upload progress. Two details worth a look:

It sends the content type the URL was signed with, not file.type. S3 checks Content-Type against the signature, so a mismatch is a 403.
It sends no Authorization header. The signature is in the URL, and an extra header would not be part of what was signed.

✔️ Verification

In apps/frontend, all green:

npm run typecheck — clean
npm run lint — clean
npx jest — 260 passed, 2 skipped, 25 suites
npm run build — static export succeeds

Coverage:

New test/lib/upload.test.ts (7) drives a stand-in XHR: the PUT's method/URL/content type, the absence of an auth header, progress reporting, ignoring non-computable progress events, and rejection on non-2xx (carrying the status), network error, and abort.
AddExpenseModal.test.tsx — asserts the upload URL is requested for the right project and filename, that receiptUrl reaches the POST body, that the PUT uses application/pdf, that the upload strictly precedes the POST, and that a failed upload records nothing and calls no onSuccess.
FileUpload.test.tsx — replaces the two fake-timer tests for the simulated upload with the controlled-progress behaviour: onChange fires immediately on drop, no progress bar appears on drop, real transferred bytes render as a percentage, and clearing progress returns to the preview.

🏕️ (Optional) Future Work / Notes

Merge feat(expenditures): presigned S3 uploads for expenditure receipts #306 first, or GET /expenditures/upload-url 404s.
The existing expenditures list has no receipt column, so uploaded receipts are not yet viewable anywhere — worth a follow-up now that receipt_url will actually be populated.

🤖 Generated with Claude Code

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, with a standing TODO to use real progress. Nothing was transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL despite the form refusing to submit without a receipt. Submitting the form now requests a presigned PUT from GET /expenditures/upload-url, sends the file to S3 with real byte progress, and passes the returned object URL to POST /expenditures as receiptUrl. If the upload fails, no expenditure is recorded and the error is shown. The upload runs on submit rather than on drop because the presigned URL is per project -- the project may not be chosen when the file lands -- and because a file dropped into an abandoned form would otherwise orphan an object in the bucket. FileUpload is now controlled: it reports the dropped file immediately and renders whatever progress its parent passes down. putWithProgress uses XHR because fetch exposes no upload progress, and sends the content type the URL was signed with rather than file.type, which S3 checks against the signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

nourshoreibah · 2026-08-04T00:30:09Z

⚠️No CI will run on this PR while it is stacked. The workflows trigger on pull_request: branches: [main, develop], which matches the base branch — so a PR based on a feature branch gets no checks at all, including the frontend-ci / lambda-tests / terraform-plan-summary that branch protection requires.

GitHub retargets this to main automatically once the parent merges and its branch is deleted; CI runs from that point. Until then the verification in the description is local only (typecheck, lint, full jest suite, production build — all green).

* fix: address audit findings across lambdas, frontend and infra Fixes the actionable findings from a full-repo bug scan. Findings already covered by open PRs (#301-#307) are deliberately untouched. Security / data exposure: - Lock down the generated-reports S3 bucket. All four block_public_* were false and a bucket policy granted s3:GetObject to Principal "*", so reports (member emails, donor contacts, expenditure amounts) were world-readable at predictable keys. Reports are now served only through a presigned GET. - Grant the lambda role s3:PutObject/s3:GetObject on the reports bucket. It had no S3 permissions at all, so POST /reports/generate was failing AccessDenied; GetObject is additionally required because a presigned URL carries the signer's permissions. - Validate objectUrl on POST /reports against the bucket's own host, so an arbitrary (e.g. javascript:) URL can no longer be stored and rendered. - Sanitize fileName before interpolating it into an S3 key in GET /reports/upload-url. - Stop logging every user row (emails, admin flags) to CloudWatch. Correctness: - Lowercase emails in UserValidationUtils.validateEmail. POST /users stored them verbatim while POST /auth/register looks up email.toLowerCase(), so any invite containing uppercase was permanently unclaimable (403 INVITATION_REQUIRED). - POST /auth/register now checks numUpdatedRows on the invitation claim. A no-op claim previously still returned 201, leaving a Cognito user whose sub referenced no row, which broke every later login. Same check on the auto-link path. - DELETE /users/{userId} deletes the Cognito user too, so the address can be re-invited. - PATCH /users/{userId} rejects email changes. Email is the Cognito username and nothing synced it, so a change silently broke sign-in and reset. - POST /donations returns 404 for a missing donor/project instead of 500, and accepts numeric strings for amount and ids. - GET /projects/{id}/donors selects explicit columns; selectAll() over a 3-table join collided project_id and leaked the whole project row. - Reject non-numeric path ids on the users and projects {id} routes, which reached Postgres as NaN and surfaced as 500s. Features that were half-built: - Add GET /reports/{id}/download returning a presigned URL. Reports could be generated but never retrieved; the frontend used object_url only for a format label. - Wire up bulk delete on the reports page. It was stubbed behind a stale comment claiming no DELETE endpoint existed, though DELETE /reports/{id} has been there all along. - Switch the reports page to server-side pagination and surface transient delete/download failures in a non-blocking banner. - Pass report_type through POST /reports/generate and return file_type. Cleanup: - Single canonical region-qualified S3 URL helper; the two call sites disagreed and the region-less form only resolved via a redirect. - Drop the unused DonationValidationUtils import. - Reconcile the duplicated auth DTOs (isAdmin is now required in both). Full dedup needs a packaging change, since neither shared package can resolve the other without a new cross-dependency. - Gitignore lambda.zip. Users-lambda tests now mock the Cognito SDK: CI injects a real user pool id, and the DELETE tests target seeded ashley@branch.org, so they would otherwise have issued live AdminDeleteUser calls against production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: auto-format terraform and update documentation - Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com> * chore: regenerate lambda READMEs * refactor(types): make @branch/types the single source of the auth DTOs The auth DTOs were declared twice, in shared/types/auth-types.d.ts and shared/lambda-auth/src/types.ts, and had already drifted (isAdmin was optional in one and required in the other). The previous commit only reconciled the two copies and left a "keep these in sync" comment, which is just documented duplication. shared/lambda-auth now takes a file: dependency on @branch/types and re-exports the DTOs from it, so there is exactly one declaration. @branch/types stays a dependency-free leaf, which is what keeps the edge acyclic; lambdas are unaffected because they already depend on both packages, and the types are erased at compile time so nothing reaches the bundle. Adding a dependency to shared/lambda-auth changes the resolved tree for every lambda, so all six package-lock.json files are regenerated. They were already stale: none recorded lambda-auth's devDependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(reports): bind report object keys to their project, and review fixes Addresses the four review comments on #310. The significant one: POST /reports only checked that objectUrl pointed at the reports bucket, not that the key belonged to the project being written. A caller with access to project A could register a row with project_id A and an objectUrl under reports/B/, then read project B's report back through GET /reports/{id}/download, which authorizes off report.project_id. That defeats the access control this PR set out to add. Keys are now bound to their project via a single reportKeyPrefix() helper used for both construction and validation: POST /reports rejects a key outside the project's prefix, and the download route re-checks the stored key against report.project_id before presigning. The prefix check runs after the access check so 403/404 still take precedence. All existing rows match the prefix, since both key-construction paths already used it. Also: - Clear the reports-page selection on page change. With server-side pagination selectedIds could retain rows from a previous page and bulk delete would remove them unseen. - Skip the Cognito delete when COGNITO_USER_POOL_ID is unset instead of making a call that cannot succeed, and log it as a configuration error. cognitoDeleted already reported false in this case via the InvalidParameterException path. - Re-indent the POST /donations membership and try/catch blocks, whose closing braces read as though they closed the route. One regression test covers the cross-project key rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

nourshoreibah · 2026-08-04T00:28:27Z

ℹ️ Issue

The last of the frontend's simulated data. Stacked on #306 — base that branch, not main.

📝 Description

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, under a standing TODO: update to use real progress of uploaded file. Nothing was ever transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL — despite the form refusing to submit without a receipt.

Submitting the form now:

Requests a presigned PUT from GET /expenditures/upload-url (added in feat(expenditures): presigned S3 uploads for expenditure receipts #306) for the chosen project and the dropped filename.
Sends the file straight to S3 with real byte progress.
Passes the returned objectUrl to POST /expenditures as receiptUrl.

If the upload fails, no expenditure is recorded and the error is surfaced — previously a failure was impossible because there was no upload.

Why the upload moved to submit rather than staying on drop: the presigned URL is per project, and the project may not be chosen when the file lands; and a file dropped into a form the user then abandons would leave an orphaned object in the bucket. The user still sees real progress, now while saving. FileUpload becomes controlled — it reports the dropped file immediately (so FilePreview still appears at once) and renders whatever progress its parent passes down.

src/lib/upload.ts — putWithProgress uses XMLHttpRequest because fetch exposes no upload progress. Two details worth a look:

It sends the content type the URL was signed with, not file.type. S3 checks Content-Type against the signature, so a mismatch is a 403.
It sends no Authorization header. The signature is in the URL, and an extra header would not be part of what was signed.

✔️ Verification

In apps/frontend, all green:

npm run typecheck — clean
npm run lint — clean
npx jest — 260 passed, 2 skipped, 25 suites
npm run build — static export succeeds

Coverage:

New test/lib/upload.test.ts (7) drives a stand-in XHR: the PUT's method/URL/content type, the absence of an auth header, progress reporting, ignoring non-computable progress events, and rejection on non-2xx (carrying the status), network error, and abort.
AddExpenseModal.test.tsx — asserts the upload URL is requested for the right project and filename, that receiptUrl reaches the POST body, that the PUT uses application/pdf, that the upload strictly precedes the POST, and that a failed upload records nothing and calls no onSuccess.
FileUpload.test.tsx — replaces the two fake-timer tests for the simulated upload with the controlled-progress behaviour: onChange fires immediately on drop, no progress bar appears on drop, real transferred bytes render as a percentage, and clearing progress returns to the preview.

🏕️ (Optional) Future Work / Notes

Merge feat(expenditures): presigned S3 uploads for expenditure receipts #306 first, or GET /expenditures/upload-url 404s.
The existing expenditures list has no receipt column, so uploaded receipts are not yet viewable anywhere — worth a follow-up now that receipt_url will actually be populated.

🤖 Generated with Claude Code

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, with a standing TODO to use real progress. Nothing was transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL despite the form refusing to submit without a receipt. Submitting the form now requests a presigned PUT from GET /expenditures/upload-url, sends the file to S3 with real byte progress, and passes the returned object URL to POST /expenditures as receiptUrl. If the upload fails, no expenditure is recorded and the error is shown. The upload runs on submit rather than on drop because the presigned URL is per project -- the project may not be chosen when the file lands -- and because a file dropped into an abandoned form would otherwise orphan an object in the bucket. FileUpload is now controlled: it reports the dropped file immediately and renders whatever progress its parent passes down. putWithProgress uses XHR because fetch exposes no upload progress, and sends the content type the URL was signed with rather than file.type, which S3 checks against the signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

nourshoreibah · 2026-08-04T00:30:09Z

⚠️No CI will run on this PR while it is stacked. The workflows trigger on pull_request: branches: [main, develop], which matches the base branch — so a PR based on a feature branch gets no checks at all, including the frontend-ci / lambda-tests / terraform-plan-summary that branch protection requires.

GitHub retargets this to main automatically once the parent merges and its branch is deleted; CI runs from that point. Until then the verification in the description is local only (typecheck, lint, full jest suite, production build — all green).

* fix: address audit findings across lambdas, frontend and infra Fixes the actionable findings from a full-repo bug scan. Findings already covered by open PRs (#301-#307) are deliberately untouched. Security / data exposure: - Lock down the generated-reports S3 bucket. All four block_public_* were false and a bucket policy granted s3:GetObject to Principal "*", so reports (member emails, donor contacts, expenditure amounts) were world-readable at predictable keys. Reports are now served only through a presigned GET. - Grant the lambda role s3:PutObject/s3:GetObject on the reports bucket. It had no S3 permissions at all, so POST /reports/generate was failing AccessDenied; GetObject is additionally required because a presigned URL carries the signer's permissions. - Validate objectUrl on POST /reports against the bucket's own host, so an arbitrary (e.g. javascript:) URL can no longer be stored and rendered. - Sanitize fileName before interpolating it into an S3 key in GET /reports/upload-url. - Stop logging every user row (emails, admin flags) to CloudWatch. Correctness: - Lowercase emails in UserValidationUtils.validateEmail. POST /users stored them verbatim while POST /auth/register looks up email.toLowerCase(), so any invite containing uppercase was permanently unclaimable (403 INVITATION_REQUIRED). - POST /auth/register now checks numUpdatedRows on the invitation claim. A no-op claim previously still returned 201, leaving a Cognito user whose sub referenced no row, which broke every later login. Same check on the auto-link path. - DELETE /users/{userId} deletes the Cognito user too, so the address can be re-invited. - PATCH /users/{userId} rejects email changes. Email is the Cognito username and nothing synced it, so a change silently broke sign-in and reset. - POST /donations returns 404 for a missing donor/project instead of 500, and accepts numeric strings for amount and ids. - GET /projects/{id}/donors selects explicit columns; selectAll() over a 3-table join collided project_id and leaked the whole project row. - Reject non-numeric path ids on the users and projects {id} routes, which reached Postgres as NaN and surfaced as 500s. Features that were half-built: - Add GET /reports/{id}/download returning a presigned URL. Reports could be generated but never retrieved; the frontend used object_url only for a format label. - Wire up bulk delete on the reports page. It was stubbed behind a stale comment claiming no DELETE endpoint existed, though DELETE /reports/{id} has been there all along. - Switch the reports page to server-side pagination and surface transient delete/download failures in a non-blocking banner. - Pass report_type through POST /reports/generate and return file_type. Cleanup: - Single canonical region-qualified S3 URL helper; the two call sites disagreed and the region-less form only resolved via a redirect. - Drop the unused DonationValidationUtils import. - Reconcile the duplicated auth DTOs (isAdmin is now required in both). Full dedup needs a packaging change, since neither shared package can resolve the other without a new cross-dependency. - Gitignore lambda.zip. Users-lambda tests now mock the Cognito SDK: CI injects a real user pool id, and the DELETE tests target seeded ashley@branch.org, so they would otherwise have issued live AdminDeleteUser calls against production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: auto-format terraform and update documentation - Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com> * chore: regenerate lambda READMEs * refactor(types): make @branch/types the single source of the auth DTOs The auth DTOs were declared twice, in shared/types/auth-types.d.ts and shared/lambda-auth/src/types.ts, and had already drifted (isAdmin was optional in one and required in the other). The previous commit only reconciled the two copies and left a "keep these in sync" comment, which is just documented duplication. shared/lambda-auth now takes a file: dependency on @branch/types and re-exports the DTOs from it, so there is exactly one declaration. @branch/types stays a dependency-free leaf, which is what keeps the edge acyclic; lambdas are unaffected because they already depend on both packages, and the types are erased at compile time so nothing reaches the bundle. Adding a dependency to shared/lambda-auth changes the resolved tree for every lambda, so all six package-lock.json files are regenerated. They were already stale: none recorded lambda-auth's devDependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(reports): bind report object keys to their project, and review fixes Addresses the four review comments on #310. The significant one: POST /reports only checked that objectUrl pointed at the reports bucket, not that the key belonged to the project being written. A caller with access to project A could register a row with project_id A and an objectUrl under reports/B/, then read project B's report back through GET /reports/{id}/download, which authorizes off report.project_id. That defeats the access control this PR set out to add. Keys are now bound to their project via a single reportKeyPrefix() helper used for both construction and validation: POST /reports rejects a key outside the project's prefix, and the download route re-checks the stored key against report.project_id before presigning. The prefix check runs after the access check so 403/404 still take precedence. All existing rows match the prefix, since both key-construction paths already used it. Also: - Clear the reports-page selection on page change. With server-side pagination selectedIds could retain rows from a previous page and bulk delete would remove them unseen. - Skip the Cognito delete when COGNITO_USER_POOL_ID is unset instead of making a call that cannot succeed, and log it as a configuration error. cognitoDeleted already reported false in this case via the InvalidParameterException path. - Re-indent the POST /donations membership and try/catch blocks, whose closing braces read as though they closed the route. One regression test covers the cross-project key rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

nourshoreibah · 2026-08-04T00:28:27Z

ℹ️ Issue

The last of the frontend's simulated data. Stacked on #306 — base that branch, not main.

📝 Description

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, under a standing TODO: update to use real progress of uploaded file. Nothing was ever transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL — despite the form refusing to submit without a receipt.

Submitting the form now:

Requests a presigned PUT from GET /expenditures/upload-url (added in feat(expenditures): presigned S3 uploads for expenditure receipts #306) for the chosen project and the dropped filename.
Sends the file straight to S3 with real byte progress.
Passes the returned objectUrl to POST /expenditures as receiptUrl.

If the upload fails, no expenditure is recorded and the error is surfaced — previously a failure was impossible because there was no upload.

Why the upload moved to submit rather than staying on drop: the presigned URL is per project, and the project may not be chosen when the file lands; and a file dropped into a form the user then abandons would leave an orphaned object in the bucket. The user still sees real progress, now while saving. FileUpload becomes controlled — it reports the dropped file immediately (so FilePreview still appears at once) and renders whatever progress its parent passes down.

src/lib/upload.ts — putWithProgress uses XMLHttpRequest because fetch exposes no upload progress. Two details worth a look:

It sends the content type the URL was signed with, not file.type. S3 checks Content-Type against the signature, so a mismatch is a 403.
It sends no Authorization header. The signature is in the URL, and an extra header would not be part of what was signed.

✔️ Verification

In apps/frontend, all green:

npm run typecheck — clean
npm run lint — clean
npx jest — 260 passed, 2 skipped, 25 suites
npm run build — static export succeeds

Coverage:

New test/lib/upload.test.ts (7) drives a stand-in XHR: the PUT's method/URL/content type, the absence of an auth header, progress reporting, ignoring non-computable progress events, and rejection on non-2xx (carrying the status), network error, and abort.
AddExpenseModal.test.tsx — asserts the upload URL is requested for the right project and filename, that receiptUrl reaches the POST body, that the PUT uses application/pdf, that the upload strictly precedes the POST, and that a failed upload records nothing and calls no onSuccess.
FileUpload.test.tsx — replaces the two fake-timer tests for the simulated upload with the controlled-progress behaviour: onChange fires immediately on drop, no progress bar appears on drop, real transferred bytes render as a percentage, and clearing progress returns to the preview.

🏕️ (Optional) Future Work / Notes

Merge feat(expenditures): presigned S3 uploads for expenditure receipts #306 first, or GET /expenditures/upload-url 404s.
The existing expenditures list has no receipt column, so uploaded receipts are not yet viewable anywhere — worth a follow-up now that receipt_url will actually be populated.

🤖 Generated with Claude Code

FileUpload animated a progress bar over a setInterval that counted through the file's size in 15 ticks, with a standing TODO to use real progress. Nothing was transferred: the File went into component state and was dropped on submit, so every expenditure was recorded with receipt_url NULL despite the form refusing to submit without a receipt. Submitting the form now requests a presigned PUT from GET /expenditures/upload-url, sends the file to S3 with real byte progress, and passes the returned object URL to POST /expenditures as receiptUrl. If the upload fails, no expenditure is recorded and the error is shown. The upload runs on submit rather than on drop because the presigned URL is per project -- the project may not be chosen when the file lands -- and because a file dropped into an abandoned form would otherwise orphan an object in the bucket. FileUpload is now controlled: it reports the dropped file immediately and renders whatever progress its parent passes down. putWithProgress uses XHR because fetch exposes no upload progress, and sends the content type the URL was signed with rather than file.type, which S3 checks against the signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

nourshoreibah · 2026-08-04T00:30:09Z

⚠️No CI will run on this PR while it is stacked. The workflows trigger on pull_request: branches: [main, develop], which matches the base branch — so a PR based on a feature branch gets no checks at all, including the frontend-ci / lambda-tests / terraform-plan-summary that branch protection requires.

GitHub retargets this to main automatically once the parent merges and its branch is deleted; CI runs from that point. Until then the verification in the description is local only (typecheck, lint, full jest suite, production build — all green).

* fix: address audit findings across lambdas, frontend and infra Fixes the actionable findings from a full-repo bug scan. Findings already covered by open PRs (#301-#307) are deliberately untouched. Security / data exposure: - Lock down the generated-reports S3 bucket. All four block_public_* were false and a bucket policy granted s3:GetObject to Principal "*", so reports (member emails, donor contacts, expenditure amounts) were world-readable at predictable keys. Reports are now served only through a presigned GET. - Grant the lambda role s3:PutObject/s3:GetObject on the reports bucket. It had no S3 permissions at all, so POST /reports/generate was failing AccessDenied; GetObject is additionally required because a presigned URL carries the signer's permissions. - Validate objectUrl on POST /reports against the bucket's own host, so an arbitrary (e.g. javascript:) URL can no longer be stored and rendered. - Sanitize fileName before interpolating it into an S3 key in GET /reports/upload-url. - Stop logging every user row (emails, admin flags) to CloudWatch. Correctness: - Lowercase emails in UserValidationUtils.validateEmail. POST /users stored them verbatim while POST /auth/register looks up email.toLowerCase(), so any invite containing uppercase was permanently unclaimable (403 INVITATION_REQUIRED). - POST /auth/register now checks numUpdatedRows on the invitation claim. A no-op claim previously still returned 201, leaving a Cognito user whose sub referenced no row, which broke every later login. Same check on the auto-link path. - DELETE /users/{userId} deletes the Cognito user too, so the address can be re-invited. - PATCH /users/{userId} rejects email changes. Email is the Cognito username and nothing synced it, so a change silently broke sign-in and reset. - POST /donations returns 404 for a missing donor/project instead of 500, and accepts numeric strings for amount and ids. - GET /projects/{id}/donors selects explicit columns; selectAll() over a 3-table join collided project_id and leaked the whole project row. - Reject non-numeric path ids on the users and projects {id} routes, which reached Postgres as NaN and surfaced as 500s. Features that were half-built: - Add GET /reports/{id}/download returning a presigned URL. Reports could be generated but never retrieved; the frontend used object_url only for a format label. - Wire up bulk delete on the reports page. It was stubbed behind a stale comment claiming no DELETE endpoint existed, though DELETE /reports/{id} has been there all along. - Switch the reports page to server-side pagination and surface transient delete/download failures in a non-blocking banner. - Pass report_type through POST /reports/generate and return file_type. Cleanup: - Single canonical region-qualified S3 URL helper; the two call sites disagreed and the region-less form only resolved via a redirect. - Drop the unused DonationValidationUtils import. - Reconcile the duplicated auth DTOs (isAdmin is now required in both). Full dedup needs a packaging change, since neither shared package can resolve the other without a new cross-dependency. - Gitignore lambda.zip. Users-lambda tests now mock the Cognito SDK: CI injects a real user pool id, and the DELETE tests target seeded ashley@branch.org, so they would otherwise have issued live AdminDeleteUser calls against production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: auto-format terraform and update documentation - Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com> * chore: regenerate lambda READMEs * refactor(types): make @branch/types the single source of the auth DTOs The auth DTOs were declared twice, in shared/types/auth-types.d.ts and shared/lambda-auth/src/types.ts, and had already drifted (isAdmin was optional in one and required in the other). The previous commit only reconciled the two copies and left a "keep these in sync" comment, which is just documented duplication. shared/lambda-auth now takes a file: dependency on @branch/types and re-exports the DTOs from it, so there is exactly one declaration. @branch/types stays a dependency-free leaf, which is what keeps the edge acyclic; lambdas are unaffected because they already depend on both packages, and the types are erased at compile time so nothing reaches the bundle. Adding a dependency to shared/lambda-auth changes the resolved tree for every lambda, so all six package-lock.json files are regenerated. They were already stale: none recorded lambda-auth's devDependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(reports): bind report object keys to their project, and review fixes Addresses the four review comments on #310. The significant one: POST /reports only checked that objectUrl pointed at the reports bucket, not that the key belonged to the project being written. A caller with access to project A could register a row with project_id A and an objectUrl under reports/B/, then read project B's report back through GET /reports/{id}/download, which authorizes off report.project_id. That defeats the access control this PR set out to add. Keys are now bound to their project via a single reportKeyPrefix() helper used for both construction and validation: POST /reports rejects a key outside the project's prefix, and the download route re-checks the stored key against report.project_id before presigning. The prefix check runs after the access check so 403/404 still take precedence. All existing rows match the prefix, since both key-construction paths already used it. Also: - Clear the reports-page selection on page change. With server-side pagination selectedIds could retain rows from a previous page and bulk delete would remove them unseen. - Skip the Cognito delete when COGNITO_USER_POOL_ID is unset instead of making a call that cannot succeed, and log it as a configuration error. cognitoDeleted already reported false in this case via the InvalidParameterException path. - Re-indent the POST /donations membership and try/catch blocks, whose closing braces read as though they closed the route. One regression test covers the cross-project key rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>