{{ message }}
Move asciicast renderer to iframe#37285
Draft
silverwind wants to merge 10 commits intogo-gitea:mainfrom
Draft
Conversation
asciinema-player uses WebAssembly, which is blocked by the main site CSP introduced in go-gitea#37232 (script-src lacks 'wasm-unsafe-eval'). Moving the renderer into an iframe does not help on its own because srcdoc iframes inherit the parent CSP per CSP3 §4.2.3.6. Two changes: - add 'wasm-unsafe-eval' to script-src so srcdoc iframes can load WASM - convert the asciicast renderer to the existing frontendRenderer iframe pattern (same as 3D viewer and openapi-swagger) for consistency and isolation Fixes go-gitea#37257 Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
GiteaBot
added
the
lgtm/need 2
The asciinema-player's .ap-term div still needs overflow: hidden to fix the scrollbar regression from go-gitea#26159. The rule is now scoped to the iframe via a local CSS import in the plugin. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
silverwind
changed the title
Reverts the main-site CSP change. Instead introduces
ExternalRendererOptions.SrcMethod ("src" | "srcdoc"/""):
- "srcdoc" (default, all other iframe renderers): iframe content is
fetched by JS and set via srcdoc; the iframe document inherits the
parent page's CSP per CSP3 §4.2.3.6.
- "src" (asciicast only): the parent page sets iframe.src and the
iframe document uses the response's own CSP — which allows
'wasm-unsafe-eval' for WebAssembly. Isolation is preserved by the
iframe sandbox attribute; the main site CSP stays untouched.
The e2e test now asserts the cast's rendered "test" text appears
inside the player — that only happens if WASM actually loaded.
Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
- SrcMethod doc collapsed to 2 lines mentioning CSP - Drop regression narration in e2e test and one-liner for asciicast registration - Replace switch-with-no-variable in RenderFile with an if/else ladder; move the PDF-RENDER-SANDBOX note next to the branch it actually describes - Fix a leftover typo in pdf test comment Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Member
Author
Renderers declare per-directive source additions as a map instead of
hardcoding 'wasm-unsafe-eval' in the router. Only the asciicast renderer
sets {"script-src": {"'wasm-unsafe-eval'"}}; the main site CSP and
other iframe renderers are unaffected.
Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
* 'cast' of github.com:silverwind/gitea: Fix Mermaid diagrams failing when node labels contain line breaks (go-gitea#37296) Add project column picker to issue and pull request sidebar (go-gitea#37037) Fix container auth for public instance (go-gitea#37290) Refactor frontend `tw-justify-between` layouts to `flex-left-right` (go-gitea#37291) Update Nix flake (go-gitea#37284) Workflow Artifact Info Hover (go-gitea#37100)
Member
Author
|
Made it more flexible, now the asciicast renderer passes |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses the .cast (asciicast) rendering CSP regression by moving asciicast rendering into the “frontend external renderer” iframe pipeline and allowing that iframe to be loaded via iframe.src with its own response-level CSP (including wasm-unsafe-eval).
Changes:
- Replace the legacy backend asciicast renderer + in-page player init with a frontend external-render plugin (
asciicast) that runs inside the external render iframe. - Add
ExternalRendererOptions.SrcMethod+AdditionalCSPSourcesto supportiframe.srcloading and per-renderer CSP extensions. - Update iframe init logic and E2E coverage for iframe-based asciicast rendering.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web_src/js/render/plugins/frontend-asciicast.ts | New frontend-render plugin implementation for asciicast (uses opts.contentString() + wasm-enabled CSP path). |
| web_src/js/render/plugins/frontend-asciicast.css | Removes old container sizing rules; keeps player UI regression fixes. |
| web_src/js/markup/render-iframe.ts | Adds support for data-src-method="src" to load iframe content via iframe.src. |
| web_src/js/markup/content.ts | Removes legacy asciicast in-markup initializer hook. |
| web_src/js/external-render-frontend.ts | Registers the asciicast frontend plugin for external rendering. |
| web_src/css/repo.css | Removes .asciicast padding override tied to the old non-iframe renderer. |
| web_src/css/index.css | Drops import of the old asciicast markup CSS. |
| types.d.ts | Expands asciinema-player.create() typing to support {data} / {url} variants. |
| tests/e2e/file-view-render.test.ts | Updates asciicast E2E test to validate iframe-based rendering and content display. |
| routers/web/repo/render.go | Adds buildIframeCSP and emits response CSP when iframe is loaded via src. |
| modules/markup/renderer.go | Extends ExternalRendererOptions with SrcMethod + AdditionalCSPSources. |
| modules/markup/render_test.go | Adds unit coverage for data-src-method="src" attribute output. |
| modules/markup/render.go | Emits data-src-method="src" on iframes when requested by renderer options. |
| modules/markup/external/frontend.go | Plumbs SrcMethod/CSP additions from frontendRenderer into renderer options. |
| modules/markup/external/external.go | Registers asciicast as a frontend external renderer with wasm CSP relaxation. |
| modules/markup/asciicast/asciicast.go | Removes the old backend asciicast renderer implementation. |
| main.go | Removes the legacy asciicast renderer registration import. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…e non-ASCII test - Reject SrcMethod="src" when the iframe sandbox is missing or contains allow-same-origin (panic in dev/testing, log + 500 in prod). - buildIframeCSP now emits any directive in AdditionalCSPSources, not just the baseline ones; output sorted for deterministic tests. - Restore the non-ASCII branch case in the asciicast e2e to guard the render-URL escaping path; still asserts WASM rendered the cast text. - Unit test for iframeSandboxSafeForSrc covering safe/unsafe sandboxes. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Contributor
|
Too many hacky patches, need to rewrite to a new approach. |
Member
Author
|
Please do. My verdict: |
Contributor
|
It needs a complete design globally. And, all "iframe renders" should share the same approach: only disallow "same-origin", allow all others, let them do anything they need. There shouldn't be so many "additional" options. |
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #37257.
ExternalRendererOptions.SrcMethod="src"loads the iframe viaiframe.srcso it uses its own response CSP instead of inheriting the parent's (per CSP3 §4.2.3.6, srcdoc iframes inherit)AdditionalCSPSources map[string][]stringappends per-directive sources; asciicast adds'wasm-unsafe-eval'toscript-srcRenderFilerejectsSrcMethod="src"without a safe sandbox (empty orallow-same-origin)This PR was written with the help of Claude Opus 4.7