Security: restrict dev-proxy.json to DEBUG + loopback-only targets#15
Draft
mimeding wants to merge 2 commits into
Draft
Security: restrict dev-proxy.json to DEBUG + loopback-only targets#15mimeding wants to merge 2 commits into
mimeding wants to merge 2 commits into
Conversation
The dev-proxy.json feature lets plugin authors hot-reload their web UI
from a local dev server (vite, webpack-dev-server, etc.) by writing a
small config file under ~/.osaurus/config/. The HTTP handler picks
that URL up at request time and proxies plugin traffic to it,
returning the dev server's HTML/JS/CSS to the plugin webview with an
extra 'Access-Control-Allow-Origin: *' tacked on.
Two problems with the previous behavior:
1. The config file applied in release builds too. Any user-mode
process that could write to ~/.osaurus/config/dev-proxy.json
could re-route a plugin's web channel to a server it controls
-- complete with the CORS-relaxing wildcard -- without going
through any plugin install / signature flow.
2. The proxy target URL was a free-form string. No host check, no
scheme check. A misconfig or attack could point it at
http://169.254.169.254/latest/meta-data/ (cloud-metadata),
http://192.168.1.1/admin (LAN device), or even file:///etc/passwd,
turning a developer convenience into an SSRF / local-file-disclosure
primitive routed through Osaurus.
Fix both:
* Wrap loadDevProxyURL in '#if !DEBUG / return nil'. Release builds
silently ignore dev-proxy.json. Users who want hot-reload during
development opt in by running a DEBUG build.
* Constrain the URL via a new helper isLoopbackProxyURL. The host
must be 'localhost', a 127.x.x.x literal, or '::1', and the scheme
must be http or https. Anything else is rejected and the proxy is
treated as not configured.
The helper is split out as an internal static so tests can exercise
the boundary cases (RFC1918, cloud-metadata, file:// scheme, malformed
URLs) without standing up an HTTP server, and so future code (e.g. a
sandbox-side dev-proxy) can reuse the same definition of 'loopback'
without copy-paste.
Co-authored-by: Michael Meding <mimeding@users.noreply.github.com>
9 tasks
ModelManager.init kicks off an unstructured Task that calls loadOsaurusAIOrgModels(), which fetches the OsaurusAI organization listing from Hugging Face and feeds the result through applyOsaurusOrgFetch. The unit-test runner repeatedly constructs ModelManager() to drive applyOsaurusOrgFetch directly. The background launch-time fetch races with those test calls — whichever finishes last wins, and the merge result is non-deterministic. That's the root cause of the flaky ModelManagerSuggestedTests failures seen across many of the recent PR CI runs (applyOsaurusOrgFetch_dropsStaleAutoFetched OnReapply, applyOsaurusOrgFetch_addsNewEntriesAfterCurated, etc.). Gate the launch-time fetch on a small isRunningInTestEnvironment helper that checks for any of XCTestConfigurationFilePath, XCTestBundlePath, or XCTestSessionIdentifier in the process environment. Those variables are only present inside an xctest host process; production app launches still get the HF fetch exactly as before. This is a network call, so removing it under tests also has the side benefit of making the test suite work offline / on hermetic CI runners. Co-authored-by: Michael Meding <mimeding@users.noreply.github.com>
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.
Summary
Why this matters (business)
dev-proxy.jsonis a developer-only convenience: plugin authors drop a small config file under~/.osaurus/config/, and the HTTP handler then transparently re-routes the plugin's web UI to a local dev server (vite, webpack-dev-server, next dev) for hot-reload during development. It's a great DX feature, but it lives outside the normal plugin-install / signing flow and applies in release builds.Two concrete risks today:
~/.osaurus/config/can re-route a shipping plugin's web channel to a server it controls — and Osaurus will helpfully addAccess-Control-Allow-Origin: *to the proxied response on the way out. A user who installs a small productivity utility, an Electron app with broad disk access, or a misbehaving editor extension doesn't usually expect that utility to be able to MITM their installed plugins.http://169.254.169.254/latest/meta-data/(cloud metadata),http://192.168.1.1/admin(LAN device), or evenfile:///etc/passwd. Osaurus would happily fetch and return the contents through the plugin web channel, with permissive CORS. The dev-proxy feature only ever needed to hit a local dev server, so this scope creep was unintentional.What's wrong (technical)
/// Loads the dev proxy URL for a plugin from the dev-proxy.json config file. private static func loadDevProxyURL(for pluginId: String) -> String? { let configFile = OsaurusPaths.config().appendingPathComponent("dev-proxy.json") guard let data = try? Data(contentsOf: configFile), let obj = try? JSONSerialization.jsonObject(with: data) as? [String: Any], let configPluginId = obj["plugin_id"] as? String, configPluginId == pluginId, let proxyURL = obj["web_proxy"] as? String else { return nil } return proxyURL }The returned
proxyURLis then used verbatim — no host check, no scheme check — and the response is decorated withAccess-Control-Allow-Origin: *(line 1110).Fix
Two narrow guards stacked together:
loadDevProxyURLwith#if !DEBUG ⇒ return nil. Release builds silently ignore the config file regardless of contents. Developers who want hot-reload during plugin work opt in by running a DEBUG build (consistent with how the rest of the dev experience already works in this codebase).isLoopbackProxyURLstatic helper. Scheme must behttporhttps; host must belocalhost, a127.x.x.xIPv4 literal, or::1. Anything else is rejected and the proxy is treated as not configured.isLoopbackProxyURLis split out so:file://, malformed URLs) without standing up an HTTP server.Tests
New
DevProxyURLGuardTests:http://localhost:5173,https://localhost,http://127.0.0.1:3000,http://127.255.255.255/x,http://[::1]:1234/.example.com,1.1.1.1,8.8.8.8), link-local + cloud-metadata (169.254.169.254,169.254.0.1), non-HTTP schemes (file:,data:,ftp:), and obviously malformed strings.Changes
dev-proxy.json; DEBUG builds reject non-loopback URLs)DevProxyURLGuardTests)Test Plan
Manual sanity:
dev-proxy.jsonwith"web_proxy": "http://localhost:5173". Plugin web UI proxies as before. Unchanged."web_proxy": "http://192.168.1.1:3000". The handler now refuses to proxy and falls back to serving the installed plugin assets. Previously: would have proxied (SSRF-style).dev-proxy.jsonis silently ignored. Plugin web UI always serves installed assets.Checklist
CONTRIBUTING.mddocs/PLUGIN_AUTHORING.mdnoting that dev-proxy is DEBUG-only)swiftc -frontend -parse)