Skip to content

fix findings from the casa security self-check - #86

Closed
hkonsti wants to merge 6 commits into
mainfrom
hkonsti/google-7-security
Closed

hkonsti wants to merge 6 commits into
mainfrom
hkonsti/google-7-security

Conversation

@hkonsti

@hkonsti hkonsti commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Security self-check for Google's CASA assessment (AL1), on the repository only: no requests were made to the hosted service, and the DAST run is left for a staging instance.

Scanned with

  • Semgrep 1.136.0: p/owasp-top-ten, p/typescript, p/nodejs, p/secrets, plus p/default, p/react, p/jwt. 10 findings: one fixed here, the rest false positives or filed.
  • gitleaks 8.30.1 on the git history and the working tree. All matches are false positives: the documented dev key, PostHog's public ingestion key, old mock data and local build caches.
  • bun audit: two transitive advisories. One has no published fix and is reached only from a dev tool. The other needs Prisma 7.
  • A hand review against the ASVS items in the verification prep (cookies, headers, logs, sessions, the OAuth server, authorization, input validation, SSRF, dependencies), and the repo's security-review skill on this branch, which found nothing.

Fixed here

  • Google sign-in no longer stores the access, refresh and id tokens it never reads. The Account row keeps only the identity.
  • The api now sends nosniff, Referrer-Policy, X-Frame-Options: DENY, frame-ancestors 'none' and, over https, HSTS on every response, and stops sending X-Powered-By. The docs and the website get the same headers from render.yaml.
  • The AES-GCM tag length is pinned when decrypting stored secrets.
  • A malformed Basic header on /oauth/token now gets invalid_client instead of a 500.
  • Tool params accept only a tool's own param names, not inherited ones like constructor.

Filed for later

  • A migration to clear and drop the Account token columns, then the privacy policy's sentence about them
  • Rate limiting on sign-in, agent login, the OAuth endpoints and MCP batches
  • Account deletion in the app
  • Pruning of the activity log and of expired sessions, codes and grants
  • A full content security policy for the web app
  • The SSRF guard for the email connector's IMAP and SMTP hosts
  • Hashing human session tokens at rest
  • Logging fewer error causes, and less of the activity log's titles and SQL
  • Pinning CI actions and the Docker base image
  • DAST with OWASP ZAP and the Fluid Attacks SAST, run by the maintainer against staging

Typecheck, lint and all 419 tests pass.

Stack: verification prep 2 of 2, on #85.


Open workspace in Conductor

@hkonsti
hkonsti added this pull request to stack #89 October 13, 2026 21:06
@hkonsti
hkonsti force-pushed the hkonsti/google-7-security branch from bec149f to 9421e73 Compare October 14, 2026 21:24
@hkonsti
hkonsti force-pushed the hkonsti/google-7-security branch from 9421e73 to 9db8204 Compare October 14, 2026 21:32
Base automatically changed from hkonsti/google-6-privacy-terms to main October 14, 2026 21:34
Google sign-in only needs the profile in the id_token, and nothing ever read the access, refresh
and id tokens it wrote to the Account row in plain text. Parse only the id_token from Google's
answer and write the identity alone; the columns stay empty for new accounts, and clearing the
existing rows and dropping the columns is left to a migration.
Only the web app's files had nosniff, a referrer policy and a framing rule; the api's JSON, its
redirects and its 404s had none, and Express announced itself. Set them once in front of
everything, deny framing everywhere (nothing frames the web app) with frame-ancestors as well,
and add HSTS where APP_URL is https, since neither Render nor Caddy sends it.
The docs and the website are static sites on Render, which adds none of nosniff, a referrer
policy, a framing rule or HSTS. Set them in the blueprint, as the app sets them itself.
Without authTagLength, node's decipher takes a tag as short as four bytes, which is far easier
to forge; Bun already refuses one, so this states the rule instead of relying on the runtime,
and a test keeps it. Flagged by semgrep's gcm-no-tag-length.
…en request

decodeURIComponent throws on a malformed percent sequence, which escaped the Do block and became
a 500 with a logged stack. Treat it as no credentials, so the client gets invalid_client.
properties[key] found constructor and toString on the prototype, so a tool appeared to take them;
parseRecordValues refused them later, but the check meant to say which params a tool takes
should say it itself.

hkonsti commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Replaced by #147, redone on current main with every check run again.

@hkonsti hkonsti closed this Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant