csp: followup fixes/dx/tweaks (#4159)

couple interesting things here as followups to the CSP work.
- first of all, again, good call on doing the report-only to start with
@SomeHats 🤘
- I combed through all the Sentry logs, looking for issues. a lot of
them were browser extensions and could be ignored.
- there were some other ones that needed fixing up though.

fixes in this PR:
- [x] CSP emulation in dev: make sure it's running in development so
that we can catch things locally. this is done via the meta tag.
- [x] `connect-src` add `blob`: this was breaking copy/export as svg/png
- [x] image testing: expand list of pasted image extensions to include
avif and some others
- [x] image pasting: this didn't really work in the first place because
typically even with CSP disabled, you'll mainly run into CORS issues. I
think it's a pretty crap user experience. So, I moved this logic to
actually be in the URL unfurling. Lemme know what you think! I don't
think we should proxy the actual image data - that sounds ... intense 😬
even though it would produce a better user experience technically.
- [x] investigated `manifest-src` errors: but it actually seems fine?
Weird thing here is that `manifest-src` isn't explicitly in the CSP so
it falls back to the `default-src` of `self` which is fine. Trying it on
tldraw.com it seems just fine with no errors but inexplicably some users
are hitting these errors. I'm guessing maybe it's an ad-blocker type
behavior maybe.
- [x] `font-src` add `data`: I'm actually unsure if this is quite
necessary but I _think_ embedded fonts in SVGs are causing the problem.
However, I can't reproduce this, I just don't mind adding this.


Before / After for pasting image URLs (not a CSP issue, to be clear, but
a CORS issue)

## Before
<img width="448" alt="Screenshot 2024-07-12 at 17 59 42"
src="https://github.com/user-attachments/assets/e8ce267b-48fd-49cd-b0f7-0fd20c0b9a1d">

## After
<img width="461" alt="Screenshot 2024-07-12 at 18 00 06"
src="https://github.com/user-attachments/assets/9956590d-fe37-4708-bc26-0c454f8151b4">

### Change type

- [ ] `bugfix`
- [ ] `improvement`
- [ ] `feature`
- [ ] `api`
- [x] `other`

### Release notes

- Security: more CSP work on dotcom
This commit is contained in:
Mime Čuvalo 2024-07-16 15:20:38 +01:00 committed by GitHub
parent e784d3182f
commit 6ba3fb0722
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 67 additions and 48 deletions

View file

@ -8,30 +8,9 @@ import json5 from 'json5'
import { nicelog } from '../../../scripts/lib/nicelog' import { nicelog } from '../../../scripts/lib/nicelog'
import { T } from '@tldraw/validate' import { T } from '@tldraw/validate'
import { csp } from '../src/utils/csp'
import { getMultiplayerServerURL } from '../vite.config' import { getMultiplayerServerURL } from '../vite.config'
const cspDirectives: { [key: string]: string[] } = {
'default-src': [`'self'`],
'connect-src': [
`'self'`,
`ws:`,
`wss:`,
`https://assets.tldraw.xyz`,
`https://*.tldraw.workers.dev`,
`https://*.ingest.sentry.io`,
],
'font-src': [`'self'`, `https://fonts.googleapis.com`, `https://fonts.gstatic.com`],
'frame-src': [`https:`],
'img-src': [`'self'`, `http:`, `https:`, `data:`, `blob:`],
'media-src': [`'self'`, `http:`, `https:`, `data:`, `blob:`],
'style-src': [`'self'`, `'unsafe-inline'`, `https://fonts.googleapis.com`],
'report-uri': [process.env.SENTRY_CSP_REPORT_URI ?? ``],
}
const csp = Object.keys(cspDirectives)
.map((directive) => `${directive} ${cspDirectives[directive].join(' ')}`)
.join('; ')
const commonSecurityHeaders = { const commonSecurityHeaders = {
'Strict-Transport-Security': 'max-age=63072000; includeSubDomains; preload', 'Strict-Transport-Security': 'max-age=63072000; includeSubDomains; preload',
'X-Content-Type-Options': 'nosniff', 'X-Content-Type-Options': 'nosniff',

View file

@ -1,5 +1,6 @@
import { Helmet } from 'react-helmet-async' import { Helmet } from 'react-helmet-async'
import { isPreviewEnv, isStagingEnv } from '../../utils/env' import { csp } from '../../utils/csp'
import { isDevelopmentEnv, isPreviewEnv, isStagingEnv } from '../../utils/env'
const showStagingFavicon = isStagingEnv || isPreviewEnv const showStagingFavicon = isStagingEnv || isPreviewEnv
@ -22,6 +23,8 @@ export function Head() {
rel="shortcut icon" rel="shortcut icon"
href={showStagingFavicon ? '/staging-favicon.svg' : '/favicon.svg'} href={showStagingFavicon ? '/staging-favicon.svg' : '/favicon.svg'}
/> />
{/* In development, we don't have the HTTP headers for CSP. We emulate it here so that we can discover things locally. */}
{isDevelopmentEnv && <meta httpEquiv="Content-Security-Policy" content={csp} />}
</Helmet> </Helmet>
) )
} }

View file

@ -0,0 +1,23 @@
export const cspDirectives: { [key: string]: string[] } = {
'default-src': [`'self'`],
'connect-src': [
`'self'`,
`ws:`,
`wss:`,
'blob:',
`https://assets.tldraw.xyz`,
`https://*.tldraw.workers.dev`,
`https://*.ingest.sentry.io`,
],
'font-src': [`'self'`, `https://fonts.googleapis.com`, `https://fonts.gstatic.com`, 'data:'],
'frame-src': [`https:`],
'img-src': [`'self'`, `http:`, `https:`, `data:`, `blob:`],
'media-src': [`'self'`, `http:`, `https:`, `data:`, `blob:`],
'style-src': [`'self'`, `'unsafe-inline'`, `https://fonts.googleapis.com`],
'style-src-elem': [`'self'`, `'unsafe-inline'`, `https://fonts.googleapis.com`],
'report-uri': [process.env.SENTRY_CSP_REPORT_URI ?? ``],
}
export const csp = Object.keys(cspDirectives)
.map((directive) => `${directive} ${cspDirectives[directive].join(' ')}`)
.join('; ')

View file

@ -1,6 +1,16 @@
import { load } from 'cheerio' import { load } from 'cheerio'
export async function unfurl(url: string) { export async function unfurl(url: string) {
// Let's see if this URL was an image to begin with.
if (url.match(/\.(a?png|jpe?g|gif|svg|webp|avif)$/i)) {
return {
title: undefined,
description: undefined,
image: url,
favicon: undefined,
}
}
const response = await fetch(url) const response = await fetch(url)
if (response.status >= 400) { if (response.status >= 400) {
throw new Error(`Error fetching url: ${response.status}`) throw new Error(`Error fetching url: ${response.status}`)
@ -9,6 +19,14 @@ export async function unfurl(url: string) {
if (!contentType?.includes('text/html')) { if (!contentType?.includes('text/html')) {
throw new Error(`Content-type not right: ${contentType}`) throw new Error(`Content-type not right: ${contentType}`)
} }
if (contentType?.startsWith('image/')) {
return {
title: undefined,
description: undefined,
image: url,
favicon: undefined,
}
}
const content = await response.text() const content = await response.text()
const $ = load(content) const $ = load(content)

View file

@ -4066,7 +4066,7 @@ export class Editor extends EventEmitter<TLEventMap> {
steppedScreenScale, steppedScreenScale,
dpr, dpr,
networkEffectiveType, networkEffectiveType,
shouldResolveToOriginal: shouldResolveToOriginal, shouldResolveToOriginal,
}) })
} }
/** /**

View file

@ -16,7 +16,7 @@ import { Dispatch, SetStateAction, useCallback, useRef, useState } from 'react'
* ``` * ```
* *
* The problem with this is that when initially mounting in strict mode, react will: * The problem with this is that when initially mounting in strict mode, react will:
* - Call the initial effect and set state state with an instance * - Call the initial effect and set state with an instance
* - Call the cleanup function and destroy the instance * - Call the cleanup function and destroy the instance
* - Call the effect again and set state with a new instance * - Call the effect again and set state with a new instance
* - Restore the state to the first instance * - Restore the state to the first instance

View file

@ -1,7 +1,7 @@
import { Editor, TLExternalContentSource, VecLike, fetch } from '@tldraw/editor' import { Editor, TLExternalContentSource, VecLike, fetch } from '@tldraw/editor'
/** /**
* When the clipboard has a file, create an image shape from the file and paste it into the scene * When the clipboard has a file, create an image/video shape from the file and paste it into the scene.
* *
* @param editor - The editor instance. * @param editor - The editor instance.
* @param urls - The file urls. * @param urls - The file urls.

View file

@ -1,5 +1,4 @@
import { Editor, TLExternalContentSource, VecLike, fetch } from '@tldraw/editor' import { Editor, TLExternalContentSource, VecLike } from '@tldraw/editor'
import { pasteFiles } from './pasteFiles'
/** /**
* When the clipboard has plain text that is a valid URL, create a bookmark shape and insert it into * When the clipboard has plain text that is a valid URL, create a bookmark shape and insert it into
@ -16,25 +15,6 @@ export async function pasteUrl(
point?: VecLike, point?: VecLike,
sources?: TLExternalContentSource[] sources?: TLExternalContentSource[]
) { ) {
// Lets see if its an image and we have CORS
try {
// skip this step if the url doesn't contain an image extension, treat it as a regular bookmark
if (new URL(url).pathname.match(/\.(png|jpe?g|gif|svg|webp)$/i)) {
const resp = await fetch(url, {
method: 'HEAD',
})
if (resp.headers.get('content-type')?.match(/^image\//)) {
editor.mark('paste')
pasteFiles(editor, [url])
return
}
}
} catch (err: any) {
if (err.message !== 'Failed to fetch') {
console.error(err)
}
}
editor.mark('paste') editor.mark('paste')
return await editor.putExternalContent({ return await editor.putExternalContent({

View file

@ -45,16 +45,28 @@ class IconExtractor {
} }
export async function getUrlMetadata({ url }: { url: string }) { export async function getUrlMetadata({ url }: { url: string }) {
// Let's see if this URL was an image to begin with.
if (url.match(/\.(a?png|jpe?g|gif|svg|webp|avif)$/i)) {
return {
title: undefined,
description: undefined,
image: url,
favicon: undefined,
}
}
const meta$ = new MetaExtractor() const meta$ = new MetaExtractor()
const title$ = new TextExtractor() const title$ = new TextExtractor()
const icon$ = new IconExtractor() const icon$ = new IconExtractor()
let response: Response
try { try {
response = (await fetch(url)) as any
await new HTMLRewriter() await new HTMLRewriter()
.on('meta', meta$) .on('meta', meta$)
.on('title', title$) .on('title', title$)
.on('link', icon$) .on('link', icon$)
.transform((await fetch(url)) as any) .transform(response)
.blob() .blob()
} catch { } catch {
return null return null
@ -75,6 +87,10 @@ export async function getUrlMetadata({ url }: { url: string }) {
favicon = new URL(favicon, url).href favicon = new URL(favicon, url).href
} }
if (response.headers.get('content-type')?.startsWith('image/')) {
image = url
}
return { return {
title, title,
description, description,