Epic 1 hygiene - #166
Conversation
Create SafeUrlValidatorService for http/https validation. Create DomainOriginVerificationService for strict iframe networking validation.
Wire bypassSecurityTrustResourceUrl inside rendered-frame.ts through the rigorous URL validator. Swap the check in preview-bridge.ts for the strict origin verification. Fix LLM concurrency in ChatCoordinator submitPrompt. Fix Monaco model.dispose() in a2ui-composer-monaco-editor. Inject takeUntilDestroyed in api-key-selector.ts and renderer-selector.ts.
Delete debug/mock-rules/ entirely. Rip out console.error patch hacks from ChatCoordinator. Delete obsolete storage key ACTIVE_DRAFT.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces origin verification for the preview bridge, adds safe URL validation for the rendered frame, cleans up the deprecated mock rules component, and improves resource disposal and subscription management in the Monaco editor and settings selectors. The review feedback highlights critical security vulnerabilities in the origin verification logic—specifically, the unsafe acceptance of 'null' origins and unvalidated query parameters. Additionally, it recommends replacing the deprecated 'substr()' method with 'slice()' and retaining detailed validation error logs in the chat coordinator to avoid hindering debugging.
| if (window.location.search) { | ||
| try { | ||
| const params = new URLSearchParams(window.location.search); | ||
| const expectedOrigin = params.get('origin'); | ||
| if (expectedOrigin && eventOrigin === expectedOrigin) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Trusting the origin query parameter directly without validating it against an allowlist or a trusted domain pattern is a security vulnerability. An attacker can frame this application, pass their own origin in the query parameter (e.g., ?origin=https://attacker.com), and bypass the origin check entirely. Please validate that expectedOrigin belongs to a trusted domain (such as localhost or your organization's trusted domains) before performing the comparison.
if (window.location.search) {
try {
const params = new URLSearchParams(window.location.search);
const expectedOrigin = params.get('origin');
const isTrustedDomain = expectedOrigin && (
expectedOrigin.startsWith("http://localhost:") ||
expectedOrigin.endsWith(".google.com") ||
expectedOrigin.endsWith(".google")
);
if (isTrustedDomain && eventOrigin === expectedOrigin) {
return true;
}
} catch {
⚡ A2UI Composer PR PreviewYour automated preview is successfully live (commit
|
1 participant
Description
First in a series of refactorings, intending to clean up the A2UI Composer code.
Security Hardening, Global Hygiene & Lifecycle Cleanup
Pre-launch Checklist