Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| cspCleanupRef.current = setupCspInterceptor( | ||
| iframe.contentWindow, | ||
| iframe.contentDocument, | ||
| (origin, category) => { | ||
| addCspObservedDomain(tool.name, origin, categoryMap[category]); | ||
| }, | ||
| ); | ||
|
|
||
| iframe.contentDocument.open(); | ||
| iframe.contentDocument.write(injectWaitForOpenai(html)); | ||
| iframe.contentDocument.close(); |
There was a problem hiding this comment.
document.open() — never fires
setupCspInterceptor is called at line 80 with the current contentDocument, but document.open() at line 88 detaches the <html> element that observeDOM passed to MutationObserver.observe(). The observer now targets a disconnected node, so no DOM mutations will fire for the actual widget content. The initial querySelectorAll inside observeDOM also runs against the blank document (no elements yet).
The practical gap: <iframe> elements emitted by document.write() (the frameDomains category) won't be captured, because they don't appear in PerformanceObserver resource entries either (iframes generate navigation entries, not resource entries).
The JSDoc on setupCspInterceptor already hints at the fix — it mentions "call the returned observeAfterWrite after document.close()" — but that callback is never returned. The DOM observation phase should be deferred to run after document.close(), e.g. by splitting the return value into { cleanup, observeAfterWrite } and calling observeAfterWrite() at line 91.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/devtools/src/components/layout/tool-panel/widget/widget.tsx
Line: 80-90
Comment:
**DOM observer set up before `document.open()` — never fires**
`setupCspInterceptor` is called at line 80 with the current `contentDocument`, but `document.open()` at line 88 detaches the `<html>` element that `observeDOM` passed to `MutationObserver.observe()`. The observer now targets a disconnected node, so no DOM mutations will fire for the actual widget content. The initial `querySelectorAll` inside `observeDOM` also runs against the blank document (no elements yet).
The practical gap: `<iframe>` elements emitted by `document.write()` (the `frameDomains` category) won't be captured, because they don't appear in PerformanceObserver resource entries either (iframes generate navigation entries, not resource entries).
The JSDoc on `setupCspInterceptor` already hints at the fix — it mentions "call the returned `observeAfterWrite` after document.close()" — but that callback is never returned. The DOM observation phase should be deferred to run after `document.close()`, e.g. by splitting the return value into `{ cleanup, observeAfterWrite }` and calling `observeAfterWrite()` at line 91.
How can I resolve this? If you propose a fix, please make it concise.| function originMatchesDomain(origin: string, domain: string): boolean { | ||
| try { | ||
| const originHost = new URL(origin).hostname; | ||
| const domainHost = new URL(domain).hostname; | ||
| return originHost === domainHost; | ||
| } catch { | ||
| return origin === domain; | ||
| } | ||
| } |
There was a problem hiding this comment.
extractOrigin in csp-interceptor.ts always produces full origins ("https://api.example.com"). If a developer configures a CSP domain as a bare hostname ("api.example.com" — valid in the ext-apps spec), new URL("api.example.com") throws and the fallback does an exact-string comparison: "https://api.example.com" === "api.example.com" → false. Every such domain will be permanently shown as "missing" regardless of configuration.
Consider normalising the domain side to accept bare hostnames by prepending a dummy scheme when the URL parse fails:
function originMatchesDomain(origin: string, domain: string): boolean {
try {
const originHost = new URL(origin).hostname;
let domainHost: string;
try {
domainHost = new URL(domain).hostname;
} catch {
// bare hostname — no protocol
domainHost = new URL(`https://${domain}`).hostname;
}
return originHost === domainHost;
} catch {
return origin === domain;
}
}Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/devtools/src/components/layout/tool-panel/csp-inspector.tsx
Line: 49-57
Comment:
**Bare-hostname CSP entries always fail to match observed origins**
`extractOrigin` in `csp-interceptor.ts` always produces full origins (`"https://api.example.com"`). If a developer configures a CSP domain as a bare hostname (`"api.example.com"` — valid in the ext-apps spec), `new URL("api.example.com")` throws and the fallback does an exact-string comparison: `"https://api.example.com" === "api.example.com"` → `false`. Every such domain will be permanently shown as "missing" regardless of configuration.
Consider normalising the `domain` side to accept bare hostnames by prepending a dummy scheme when the URL parse fails:
```ts
function originMatchesDomain(origin: string, domain: string): boolean {
try {
const originHost = new URL(origin).hostname;
let domainHost: string;
try {
domainHost = new URL(domain).hostname;
} catch {
// bare hostname — no protocol
domainHost = new URL(`https://${domain}`).hostname;
}
return originHost === domainHost;
} catch {
return origin === domain;
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
fredericbarthelet
force-pushed
the
fred/add-csp-inspector
branch
4 times, most recently
from
3e90427 to
a512a40
Compare
fredericbarthelet
force-pushed
the
fred/add-csp-inspector
branch
from
a512a40 to
160088f
Compare
1 participant
Just a POC to demo the _meta.ui.csp metadata confirmation of ext-apps and how Skybridge can help developers inspect and correct missing/extra domains in CSPs.
Greptile Summary
This POC adds a CSP Inspector tab to the devtools panel that intercepts network activity from widget iframes (via fetch/XHR monkey-patching, PerformanceObserver, and DOM mutation observation), compares observed origins against the declared
_meta.ui.cspconfig, and surfaces missing domains with a copyable suggested snippet.setupCspInterceptoris invoked beforedocument.open(), so the MutationObserver is attached to a node thatdocument.open()immediately detaches — the observer never fires. This silently drops allframeDomainsdetection for static<iframe>elements (not covered by PerformanceObserver either). The stale JSDoc onsetupCspInterceptorreferences a non-existentobserveAfterWritereturn that was apparently the intended fix.Confidence Score: 4/5
Safe to merge as a POC, but the DOM observer is effectively a no-op, silently missing an entire detection category (frameDomains) — worth fixing before promoting beyond an experiment.
One P1 finding (DOM observer never fires due to ordering relative to document.open) leaves a functional gap in frame-domain tracking that isn't obvious from runtime behaviour. The P2 domain-matching issue can produce misleading false-positives. Both should be resolved before this ships as a real feature.
packages/devtools/src/components/layout/tool-panel/widget/widget.tsx (CSP interceptor setup order) and packages/devtools/src/lib/csp-interceptor.ts (missing observeAfterWrite phase).
Vulnerabilities
No security concerns identified. The monkey-patching of
fetchandXMLHttpRequest.prototype.openis scoped to the sandboxed iframe window and is cleaned up on unmount. No credentials, tokens, or sensitive data are captured — only request origins are recorded.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "wip" | Re-trigger Greptile