Skip to content

fix: harden oauth popup messaging - #182

Merged
mbakgun merged 1 commit into
heymrun:mainfrom
Rohithmatham12:fix/oauth-popup-message-hardening
Jun 12, 2026
Merged

fix: harden oauth popup messaging#182
mbakgun merged 1 commit into
heymrun:mainfrom
Rohithmatham12:fix/oauth-popup-message-hardening

Conversation

@Rohithmatham12

@Rohithmatham12 Rohithmatham12 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Restrict Google Sheets and BigQuery OAuth callback postMessage calls to the callback page origin instead of using a wildcard target
  • Ignore OAuth popup messages unless they come from the popup window opened by the credential dialog and from the current app origin
  • Serialize OAuth popup payloads with JSON and escape that JSON for safe inline script embedding
  • Cover callback target-origin behavior and script-breaking </script> payloads in regression tests

Testing

  • bun run lint:check
  • bun run typecheck
  • bun run build
  • uv run ruff format --check .
  • uv run ruff check .
  • git diff --check
  • SECRET_KEY=test-secret-key-for-tests-only-32-bytes ENCRYPTION_KEY=0000000000000000000000000000000000000000000000000000000000000001 uv run python -m unittest tests.test_google_sheets_oauth (19 passed)
  • SECRET_KEY=test-secret-key-for-tests-only-32-bytes ENCRYPTION_KEY=0000000000000000000000000000000000000000000000000000000000000001 ./run_tests.sh (986 passed before the follow-up inline-script escape)

@Rohithmatham12
Rohithmatham12 force-pushed the fix/oauth-popup-message-hardening branch from f8bf4c1 to dc8a3b1 Compare June 11, 2026 19:56
@mbakgun

mbakgun commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Thanks for the hardening here as well @Rohithmatham12 🚀. One issue still remains in the popup HTML generation.

The payload is now serialized with JSON, which fixes JavaScript string escaping, but it is still embedded directly inside an inline <script>. If the message contains </script>, the browser HTML parser can close the script tag before JavaScript parsing happens. For example, an error message like </script><script>window.__xss = true</script> is emitted raw in the callback HTML.

Could you also escape the JSON for safe inline script embedding, for example by replacing < with \u003c after json.dumps, and add a regression test that verifies </script> is not present in the generated HTML payload?

This may have existed before this PR, but since the PR is hardening this exact popup messaging path, it would be good to close it here. 🤗

@Rohithmatham12
Rohithmatham12 force-pushed the fix/oauth-popup-message-hardening branch from dc8a3b1 to 61068a0 Compare June 11, 2026 22:42

Copy link
Copy Markdown
Contributor Author

Thanks for catching that. I updated both OAuth popup helpers to escape the JSON after serialization before embedding it in the inline script, covering <, >, &, U+2028, and U+2029. I also added regression coverage for the </script><script>... payload on both Google Sheets and BigQuery popup HTML generation.

Reran lint, typecheck, build, ruff format/check, diff check, and the targeted OAuth test module.

@mbakgun
mbakgun merged commit eb76f50 into heymrun:main Jun 12, 2026
1 check passed
@mbakgun

mbakgun commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Thx @Rohithmatham12 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants