Follow-up to #3 (completed fixes, PRs #1 and #2). These are the items from the Oct 2026 security review that are still open.
Blocked on rollout
Remove legacy ?key= staff auth. Do this only after extension 0.4.0 is on both client.crm.fci.coop and crm.fci.coop. Delete the legacy branch in lib/staff-auth.ts and drop key from /staff/report, /api/staff/report and /api/staff/file. Before removing it, check the Amplify logs for the [staff-auth] legacy ?key= access used warning to confirm nothing still sends the old links.
Fix the extension verification step in PRODUCTION_CUTOVER.md §5. With 0.4.0, a bare curl -X POST …/civicrm/webform-mw/upload returns {"error":"Missing X-Requested-With header"} instead of Missing file part. Any JSON error means the route is live; a 404 or login redirect means the flush didn't take.
Open findings
#7 Rate limits can be bypassed (Low–Med).clientIp() in lib/rate-limit.ts trusts the leftmost X-Forwarded-For entry, which the client controls. The in-memory buckets also don't hold up across multiple Lambda instances. Next step: capture the real forwarding headers from one production request (e.g. a temporary header dump on token-gated /api/health), then pick the right entry from what CloudFront/Amplify actually sets. Don't guess: choosing a CloudFront edge IP would put every user in one bucket. Longer term, consider AWS WAF rate-based rules in front of Amplify.
#9 Signed file URLs leave the app (Low/privacy). The staff report sends Civi's fcs-signed download URLs to the browser, and "View in Google Docs" (components/report/FileLink.tsx) sends them to Google. Each one is a bearer link until it expires. Product decision: keep the Google viewer, or drop it and route office downloads through /api/staff/file.
Defense in depth (Low)
/api/uploadsanitizeFilename() allows any extension (e.g. x.php with a PDF header passes the magic-byte check). Civi's makeFileName() neutralizes unsafe extensions, but an extension allowlist that matches the mime would add another layer.
CSP script-src 'unsafe-inline' (next.config.ts). There are no dangerouslySetInnerHTML/eval sinks today; consider nonce-based CSP if Next 16 supports it cleanly.
Least privilege for the Civi API user behind CIVI_API_KEY: review its permissions. Note that it now needs EDIT on form-filler contacts for uploads (#3, extension 0.4.0).
HEALTH_TOKEN is still accepted via ?token= on /api/health. Consider moving it to a Bearer header like preview-link.
Housekeeping
Remove render.yaml and the Render sections of DEPLOYMENT.md / AMPLIFY_DEPLOY.md. The app deploys only on AWS Amplify (client account), via the GitHub mirror.
Pre-existing lint errors (not security): AttachmentLightbox.tsx:50 (setState in effect), DateTimeline.tsx:81 and MembershipChart.tsx:149 (impure function during render).
Add a scheduled dependency audit (npm audit / osv-scanner) so the next Next.js advisory surfaces automatically.
Follow-up to #3 (completed fixes, PRs #1 and #2). These are the items from the Oct 2026 security review that are still open.
## Blocked on rollout
- [ ] **Remove legacy `?key=` staff auth.** Do this only after extension 0.4.0 is on both client.crm.fci.coop and crm.fci.coop. Delete the legacy branch in `lib/staff-auth.ts` and drop `key` from `/staff/report`, `/api/staff/report` and `/api/staff/file`. Before removing it, check the Amplify logs for the `[staff-auth] legacy ?key= access used` warning to confirm nothing still sends the old links.
- [ ] **Fix the extension verification step in `PRODUCTION_CUTOVER.md` §5.** With 0.4.0, a bare `curl -X POST …/civicrm/webform-mw/upload` returns `{"error":"Missing X-Requested-With header"}` instead of `Missing file part`. Any JSON error means the route is live; a 404 or login redirect means the flush didn't take.
## Open findings
- [ ] **#7 Rate limits can be bypassed (Low–Med).** `clientIp()` in `lib/rate-limit.ts` trusts the leftmost `X-Forwarded-For` entry, which the client controls. The in-memory buckets also don't hold up across multiple Lambda instances. **Next step:** capture the real forwarding headers from one production request (e.g. a temporary header dump on token-gated `/api/health`), then pick the right entry from what CloudFront/Amplify actually sets. Don't guess: choosing a CloudFront edge IP would put every user in one bucket. Longer term, consider AWS WAF rate-based rules in front of Amplify.
- [ ] **#9 Signed file URLs leave the app (Low/privacy).** The staff report sends Civi's `fcs`-signed download URLs to the browser, and "View in Google Docs" (`components/report/FileLink.tsx`) sends them to Google. Each one is a bearer link until it expires. *Product decision:* keep the Google viewer, or drop it and route office downloads through `/api/staff/file`.
## Defense in depth (Low)
- [ ] `/api/upload` `sanitizeFilename()` allows any extension (e.g. `x.php` with a PDF header passes the magic-byte check). Civi's `makeFileName()` neutralizes unsafe extensions, but an extension allowlist that matches the mime would add another layer.
- [ ] CSP `script-src 'unsafe-inline'` (`next.config.ts`). There are no `dangerouslySetInnerHTML`/`eval` sinks today; consider nonce-based CSP if Next 16 supports it cleanly.
- [ ] Least privilege for the Civi API user behind `CIVI_API_KEY`: review its permissions. Note that it now needs EDIT on form-filler contacts for uploads (#3, extension 0.4.0).
- [ ] `HEALTH_TOKEN` is still accepted via `?token=` on `/api/health`. Consider moving it to a Bearer header like preview-link.
## Housekeeping
- [ ] Remove `render.yaml` and the Render sections of `DEPLOYMENT.md` / `AMPLIFY_DEPLOY.md`. The app deploys only on AWS Amplify (client account), via the GitHub mirror.
- [ ] Pre-existing lint errors (not security): `AttachmentLightbox.tsx:50` (setState in effect), `DateTimeline.tsx:81` and `MembershipChart.tsx:149` (impure function during render).
- [ ] Add a scheduled dependency audit (`npm audit` / osv-scanner) so the next Next.js advisory surfaces automatically.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Follow-up to #3 (completed fixes, PRs #1 and #2). These are the items from the Oct 2026 security review that are still open.
Blocked on rollout
?key=staff auth. Do this only after extension 0.4.0 is on both client.crm.fci.coop and crm.fci.coop. Delete the legacy branch inlib/staff-auth.tsand dropkeyfrom/staff/report,/api/staff/reportand/api/staff/file. Before removing it, check the Amplify logs for the[staff-auth] legacy ?key= access usedwarning to confirm nothing still sends the old links.PRODUCTION_CUTOVER.md§5. With 0.4.0, a barecurl -X POST …/civicrm/webform-mw/uploadreturns{"error":"Missing X-Requested-With header"}instead ofMissing file part. Any JSON error means the route is live; a 404 or login redirect means the flush didn't take.Open findings
clientIp()inlib/rate-limit.tstrusts the leftmostX-Forwarded-Forentry, which the client controls. The in-memory buckets also don't hold up across multiple Lambda instances. Next step: capture the real forwarding headers from one production request (e.g. a temporary header dump on token-gated/api/health), then pick the right entry from what CloudFront/Amplify actually sets. Don't guess: choosing a CloudFront edge IP would put every user in one bucket. Longer term, consider AWS WAF rate-based rules in front of Amplify.fcs-signed download URLs to the browser, and "View in Google Docs" (components/report/FileLink.tsx) sends them to Google. Each one is a bearer link until it expires. Product decision: keep the Google viewer, or drop it and route office downloads through/api/staff/file.Defense in depth (Low)
/api/uploadsanitizeFilename()allows any extension (e.g.x.phpwith a PDF header passes the magic-byte check). Civi'smakeFileName()neutralizes unsafe extensions, but an extension allowlist that matches the mime would add another layer.script-src 'unsafe-inline'(next.config.ts). There are nodangerouslySetInnerHTML/evalsinks today; consider nonce-based CSP if Next 16 supports it cleanly.CIVI_API_KEY: review its permissions. Note that it now needs EDIT on form-filler contacts for uploads (#3, extension 0.4.0).HEALTH_TOKENis still accepted via?token=on/api/health. Consider moving it to a Bearer header like preview-link.Housekeeping
render.yamland the Render sections ofDEPLOYMENT.md/AMPLIFY_DEPLOY.md. The app deploys only on AWS Amplify (client account), via the GitHub mirror.AttachmentLightbox.tsx:50(setState in effect),DateTimeline.tsx:81andMembershipChart.tsx:149(impure function during render).npm audit/ osv-scanner) so the next Next.js advisory surfaces automatically.