Repository navigation
Fixes from the security review of the 1.5.0 changes - #221
Merged
Merged
Conversation
Python in the Work Room canvas ran in an unsandboxed frame on MATE's origin. Agent-written Python could use Pyodide's js module to act as the person who clicked Run. It now runs on the sandboxed canvas page, like HTML and JavaScript, and Open in new tab runs it there too. script-src allowed all of cdn.jsdelivr.net and cdnjs.cloudflare.com, which serve any npm package or GitHub repository, so an HTML injection could load an attacker's script under enforce. It now lists each library's path with its version, a test fails on a template script it does not cover, and 'wasm-unsafe-eval' is gone with Pyodide. OpenRouter prices must be finite and at most $0.1 per token, so a NaN cannot poison cost sums and dollar budgets, and the list is read up to 25 MB. Each Playground run writes an audit entry naming the admin and what was compared.
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.
An independent review covered everything merged since #194: CSP steps 3–6, costs and dollar budgets, the Playground, the MySQL fixes and the contributor PRs. It found nothing critical or high. This PR fixes the two medium findings and two of the low ones. The third low one, dollar budgets that don't count unpriced calls, is a design question and gets its own issue.
Python in the Work Room canvas ran unsandboxed (medium)
/dashboard/workroom/canvaspage. Python stayed in asrcdociframe withoutsandbox, on MATE's origin. The code comment called it safe because "users run their own code", but an agent writes it.jsmodule acts as whoever clicks Run. For an admin, that means creating users or reading agent configs, keys included.static/js/workroom-python.jsand sends it inline, with Pyodide, to the sandboxed canvas page, exactly like JS. The page can't load files from MATE itself on a loopback or private address (Chrome).<script>.CSP allowed whole CDN hosts for scripts (medium)
script-srcallowed all ofcdn.jsdelivr.netandcdnjs.cloudflare.com. Those serve any npm package or GitHub repository (/gh/<user>/<repo>), so any HTML injection could load an attacker's script even underCSP_MODE=enforce._SCRIPT_HOSTSnow lists each library's path with its version, as a prefix or an exact path: Chart.js, Monaco, React and React DOM, React Flow, markdown-it, Swagger UI, Redoc, Ace. Tailwind's host stays whole, since it serves only Tailwind.'wasm-unsafe-eval'is dropped from the dashboard's policy, because Pyodide no longer runs there.test_every_cdn_script_in_the_templates_is_allowedfails when a template loads a CDN script the policy doesn't cover.OpenRouter prices weren't validated (low)
float("NaN")andfloat("inf")passed the< 0check. A NaN cost makes SQL sums NaN, which blocks every request in a dollar-budget scope and breaks the alert's percentage.Playground runs weren't tied to a person (low)
evals.playground_runis written before the run, so a failed or hung run is recorded too. It names the admin and records the mode, the number of cases, and each variant's model and version. Changed instructions are recorded by length and a SHA-256 prefix, not as text.Checks
CSP_MODE=enforce, Chromium:sum 5), and the iframe hassandbox="allow-scripts allow-modals"and loads the canvas page.js.window.parent.documentandjs.localStorageboth fail./admin-docs. Each library is loaded:Chart,markdownit,ReactFlowandReact,SwaggerUIBundle,ace, and the Monaco loader.https://cdn.jsdelivr.net/gh/jquery/jquery@3.7.1/...is blocked (script-src-elem).gen_docs.py --check: OK.gen_docs.py drift:docs/dev/content-security-policy.md(script paths; Python in the sandbox; the Python canvas gap removed),docs/user/work-room.md,docs/user/settings.mdanddocs/user/evals.md;eu-ai-act.md,getting-started.md,usage-and-audit.md,deployment.mdandtemplate-format.mdare flagged only because files they cover changed, and they still hold.