Skip to content

feat(browser): isolated proxy origin + service worker (SPAs work in the Browser app)#511

Merged
jaylfc merged 7 commits into
masterfrom
feat/browser-proxy-origin
May 31, 2026
Merged

feat(browser): isolated proxy origin + service worker (SPAs work in the Browser app)#511
jaylfc merged 7 commits into
masterfrom
feat/browser-proxy-origin

Conversation

@jaylfc
Copy link
Copy Markdown
Owner

@jaylfc jaylfc commented May 31, 2026

Frontend win #2. Makes JS-heavy sites / SPAs work in the Browser app by giving the proxied iframe a real, isolated origin so a service worker can intercept its runtime fetches (which the server-side lxml rewriter can't reach).

What it does

  • Second, API-free origin on a separate port (default 6970, TAOS_BROWSER_PROXY_PORT). Serves ONLY the proxy + /__taos/sw.js + /__taos/copilot.js + /__taos/redeem (no taOS APIs — proven by tests). Graceful single-port fallback if disabled.
  • Cross-origin auth (mirrors /shortcut/): authed shell mints a 30s single-use HMAC ticket → /__taos/redeem validates it, sets an httponly/samesite=lax taos_browser cookie, redirects to the proxied page (open-redirect guarded). Prevents an open web proxy on :6970.
  • Iframe gets allow-same-origin (only when cross-origin to the shell; single-port keeps the opaque sandbox) → the SW registers from copilot.js in-iframe and intercepts the page's fetch/XHR, routing them back through the proxy. Fixes the prior silently-inoperative SW.
  • Dual-port serving in __main__.py (shared state via a whitelisted delegating wrapper); installer opens 6970.
  • Web-push fix: push/notificationclick handlers moved to the shell SW (desktop/src/sw.ts) since the shell-side proxy-SW registration was removed.

Security

Reviewed (security pass): API-free origin ✅, SSRF guards intact (private/loopback/metadata/encoded-IP blocked) ✅, no taOS cookies leak upstream ✅, HMAC/TTL/single-use/open-redirect guards ✅, allow-same-origin safe + correctly withheld single-port ✅. Plus 4 hardening fixes applied: Referrer-Policy on redeem, postMessage targets the proxy origin (not *), taos-sw:prime validated (no profile re-prime), and _SharedState attribute allowlist.

Needs a real-browser smoke before merge

jsdom can't test the SW actually intercepting cross-origin SPA fetches. On a real dual-port deploy: load a client-routed SPA in the Browser app, navigate within it, confirm sub-resource/API GETs show as …/api/desktop/browser/proxy?... in the Network tab and the taos_browser cookie rides along.

Tests: 534 backend + 21 frontend (BrowserApp) pass; tsc clean; build OK.

Summary by CodeRabbit

  • New Features

    • Web Push notifications for desktop.
    • Browser proxy available as a separate origin with ticket-based redemption.
    • Browser proxy port configurable via env var.
  • Improvements

    • Hardened service worker validation and origin handling.
    • Safer, clearer iframe proxy loading with explicit error surfacing and focused postMessage targets.
  • Chores

    • Installer now exposes proxy port configuration.
  • Tests

    • Expanded test coverage for proxy, redeem flow, and service worker.

jaylfc added 4 commits May 31, 2026 17:54
… origin

Frontend win #2 part 1 (backend foundation). A service worker can only
control clients of its own origin; the proxied iframe is currently
same-origin with the shell but sandboxed to an opaque origin, so the
shell SW never controls it. Give the iframe a real, separate origin
(default port 6970) with NO taOS APIs so allow-same-origin is safe there.

- browser_proxy_origin.create_browser_proxy_app(state): second ASGI app
  mounting only the proxy fetch + /__taos/sw.js + /__taos/copilot.js +
  a new /__taos/redeem. Shares the main app's app.state (browser cookie
  store, profile store, copilot hub, auth) by reference. Own auth gate:
  every protected request needs a valid taos_browser cookie or 401 (no
  redirect to the main login — this origin has no login UI).
- redeem flow mirrors /shortcut/: main app mints a short-lived signed
  HMAC ticket at /api/desktop/browser/proxy-ticket (authed); the proxy
  origin redeems it at /__taos/redeem, sets taos_browser, and 302s to a
  next that is validated to be an on-origin proxy path (no open redirect).
- proxy_ticket: HMAC token mint/validate + single-use JTI replay guard,
  matching shortcuts.tickets. Signing key is a per-process secret on
  shared app.state.
- dual-port serving in __main__: two uvicorn.Server under asyncio.gather.
  TAOS_BROWSER_PROXY_PORT env + server.browser_proxy_port config (default
  6970). Set to 0 to disable and degrade to single-port.

Tests: proxy origin is API-free (taOS routes 404), token-gated (401
without cookie, 200 after redeem), redeem rejects invalid/expired tickets
and off-origin next; main-app ticket endpoint requires auth.
…W + SPAs)

Open/navigate now goes through the ticketed redeem flow on the proxy origin:
each top-level navigation mints a fresh 30s single-use proxy ticket
(same-origin, credentialed) and points the iframe at
<proxyOrigin>/__taos/redeem?ticket=…&next=<encoded proxied path>. The redeem
sets the taos_browser cookie on the proxy origin and 302s to the proxied page;
in-page SPA fetches are handled by the proxy-origin service worker.

- Frontend learns the proxy port from a new public /api/desktop/browser/proxy-config
  probe and builds the origin from the current access host
  (location.protocol//hostname:port), so it works over LAN / Tailscale /
  taos.local. Port 0 (or port == main port) → single-port fallback: same-origin
  proxy URLs and the historical opaque-origin sandbox (no allow-same-origin).
- Sandbox: add allow-same-origin only when the proxy is cross-origin to the
  shell (the separate, API-free origin makes it safe and lets the proxied page
  register a service worker).
- Service worker registration + priming moved into the iframe (copilot.js on the
  proxy origin), primed via taos-page-base / taos-profile-id meta tags injected
  by the backend. Removed the shell-side /__taos/sw.js registration.
- Ticket-mint failure surfaces an inline error in the tab instead of a blank
  iframe.

Tests: TabRenderer redeem URL shape + cross/same-origin sandbox + ticket-failure
error; browser-proxy-config helpers; injector SW-prime meta; proxy-config route;
copilot.js SW registration. tsc + build clean.
…ort in installer

Port push + notificationclick handlers from the proxy SW into
desktop/src/sw.ts so web push targets the shell /sw.js registration
(where bootstrapPushSubscription resolves via navigator.serviceWorker.ready).

Add TAOS_BROWSER_PROXY_PORT (default 6970) to install-server.sh:
document the env var, thread it through systemd/launchd service configs
via the existing Environment block, and mention it in the closing log
so users know to open both ports.
…rigin, SW prime validation, SharedState allowlist)

- Redeem 302 now sets Referrer-Policy: no-referrer so single-use tickets
  don't leak via the Referer header to the proxied site
- TabRenderer postMessage calls use the resolved proxy origin instead of
  "*" so copilot tickets aren't broadcast to arbitrary origins
- SW taos-sw:prime handler validates event.source, profileId slug, and
  pageBaseUrl origin before updating globals to block re-prime attacks
- _SharedState.__getattr__ enforces an allowlist (auth, browser_cookie_store,
  browser_proxy_signing_key, browser_store, copilot_hub) and raises
  AttributeError for anything outside it

Tests added for all four fixes; 534 Python + 21 TS tests pass.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 31, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Implements a browser-proxy origin with HMAC-signed short-lived tickets, client helpers for proxy origin and redeem, in-iframe SW registration/priming, hardened priming validation, TabRenderer TabFrame redeem/sandbox logic, and dual-port server startup with installer/config changes.

Changes

Browser Proxy Origin Implementation

Layer / File(s) Summary
Backend ticket minting and configuration endpoints
tinyagentos/routes/desktop_browser/proxy_ticket.py, tinyagentos/routes/desktop_browser/proxy_ticket_routes.py, tinyagentos/config.py, data/config.yaml.example, desktop/src/lib/browser-proxy-config.ts, desktop/src/lib/browser-proxy-config.test.ts, scripts/install-server.sh, tinyagentos/routes/desktop_browser/__init__.py
Adds HMAC-SHA256-signed proxy tickets with JTI replay protection, endpoints /api/desktop/browser/proxy-config (public) and /api/desktop/browser/proxy-ticket (auth-gated), client helpers to fetch/configure proxy port and mint tickets, config default/example for browser_proxy_port, and installer propagation of TAOS_BROWSER_PROXY_PORT.
Browser proxy origin ASGI app with session & auth
tinyagentos/browser_proxy_origin.py, tests/routes/desktop_browser/test_browser_proxy_origin.py
New minimal FastAPI proxy app that serves SW/copilot assets and /__taos/redeem, maintains in-memory short-lived taos_browser sessions, sets HttpOnly SameSite=Lax cookie on redeem, enforces safe next redirects, and exposes a shared-state allowlist wrapper. Tests cover isolation, redeem flow, ticket validation, and shared-state behavior.
Desktop service worker push handling and proxy SW priming validation
desktop/src/sw.ts, desktop/src/lib/__tests__/sw-push-handlers.test.ts, tinyagentos/routes/desktop_browser/sw.js, tests/routes/desktop_browser/test_sw_asset.py
Shell SW gains push and notificationclick handlers (payload parse/fallback, same-origin client targeting/focus/open). Proxy SW priming handler is hardened: requires message source, enforces taos-sw:prime type, validates profileId pattern and pageBaseUrl origin. Tests exercise push handling and priming validation.
HTML injection and in-iframe SW priming
tinyagentos/routes/desktop_browser/injector.py, tests/routes/desktop_browser/test_injector.py, tinyagentos/routes/desktop_browser/copilot.js, tests/routes/desktop_browser/test_copilot_js.py, tinyagentos/routes/desktop_browser/proxy.py
inject_into_head now accepts page_base_url and profile_id and centralizes meta insertion via _set_meta. Copilot script registers /__taos/sw.js inside the iframe and posts taos-sw:prime with injected meta. Proxy passes upstream URL/profile into injector. Tests updated accordingly.
TabRenderer refactor: TabFrame with redeem flow and postMessage origin targeting
desktop/src/apps/BrowserApp/TabRenderer.tsx, desktop/src/apps/BrowserApp/TabRenderer.test.tsx, desktop/src/apps/BrowserApp/BrowserApp.tsx, desktop/src/lib/browser-proxy-config.ts
Replaces inline iframe with TabFrame that builds proxied paths, mints proxy tickets, constructs redeem URLs, manages about:blank loading, surfaces inline errors when minting fails, conditionally grants allow-same-origin for cross-origin proxy, and resolves postMessage target origin via getBrowserProxyOrigin(). Tests validate redeem URL structure, sandbox differences, error handling, and postMessage targets.
Server startup, installer, and config defaults
scripts/install-server.sh, tinyagentos/__main__.py, tinyagentos/auth_middleware.py, desktop/tsconfig.tsbuildinfo
Adds TAOS_BROWSER_PROXY_PORT env override and installer propagation; DEFAULT_CONFIG now includes browser_proxy_port; __main__ resolves proxy port and conditionally runs dual-port serving via _serve_dual_port; auth middleware exempts /chat-pwa; tests set TAOS_BROWSER_PROXY_PORT=0 to disable dual-port during unit tests.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Possibly related PRs

🐰 "I mint a ticket, swift and neat,
I hop through ports to hide the street.
Meta tags and primed SW cheer,
Sandbox safe and origins clear.
A tiny hop, a quiet shove — proxy magic done with love."

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.37% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main feature: adding an isolated proxy origin with service worker support to enable SPAs in the Browser app.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/browser-proxy-origin

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (6)
desktop/src/apps/BrowserApp/TabRenderer.tsx (1)

172-188: 💤 Low value

Cleanup's async getBrowserProxyOrigin() may race with the next mount.

The cleanup function fires getBrowserProxyOrigin().then(...) asynchronously without checking the cancelled flag inside the callback. If the component unmounts and remounts quickly, the old cleanup's close messages may arrive after the new effect has already sent open messages to the iframe, causing an out-of-order closeopen sequence to become openclose.

Consider capturing proxyOrigin from the effect scope and using it synchronously in the cleanup, or check cancelled inside the .then() callback.

♻️ Suggested fix: use captured proxyOrigin in cleanup
   useEffect(() => {
     if (!activeTabIdForEffect || !profileIdForEffect) return;

     const tabId = activeTabIdForEffect;
     const profileId = profileIdForEffect;
     const iframe = document.querySelector(
       `iframe[data-tab-id="${tabId}"]`,
     ) as HTMLIFrameElement | null;
     if (!iframe) return;

     const handles: AgentWsHandle[] = [];
     let cancelled = false;
+    let resolvedProxyOrigin: string | null = null;

     getBrowserProxyOrigin().then((proxyOrigin) => {
       if (cancelled) return;
+      resolvedProxyOrigin = proxyOrigin;

       for (const agentId of pinnedAgentIdsForEffect) {
         // ... existing code ...
       }
     });

     return () => {
       cancelled = true;

       for (const handle of handles) handle.close();

-      getBrowserProxyOrigin().then((proxyOrigin) => {
-        if (!iframe.contentWindow) return;
+      if (!resolvedProxyOrigin || !iframe.contentWindow) return;
       for (const agentId of pinnedAgentIdsForEffect) {
         iframe.contentWindow.postMessage(
           { type: "taos-copilot:close", agentId },
-          proxyOrigin,
+          resolvedProxyOrigin,
         );
       }
-      });
     };
   // ...
   }, [windowId, activeTabIdForEffect, pinnedAgentIdsForEffect.join(","), profileIdForEffect]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@desktop/src/apps/BrowserApp/TabRenderer.tsx` around lines 172 - 188, The
cleanup currently calls getBrowserProxyOrigin().then(...) asynchronously which
can race with a new mount; capture proxyOrigin synchronously in the effect scope
(call await/get synchronously before registering the cleanup) or guard the then
callback with the cancelled flag so the old cleanup bail outs; specifically, in
the effect that defines cancelled, handles, pinnedAgentIdsForEffect and iframe,
obtain proxyOrigin once (via getBrowserProxyOrigin()) and reference that
captured proxyOrigin inside the cleanup instead of calling
getBrowserProxyOrigin() asynchronously, or at minimum check if cancelled is true
at the start of the .then(...) callback and return early to avoid sending stale
iframe.contentWindow.postMessage close messages.
tests/routes/desktop_browser/test_browser_proxy_origin.py (1)

255-271: 💤 Low value

Test name is slightly misleading.

test_redeem_302_carries_no_referrer_policy implies the header is absent, but it actually verifies the header IS present with value no-referrer. Consider renaming to test_redeem_302_sets_no_referrer_policy for clarity.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/routes/desktop_browser/test_browser_proxy_origin.py` around lines 255 -
271, The test name test_redeem_302_carries_no_referrer_policy is misleading
because it suggests the header is absent while the assertion checks the header
equals "no-referrer"; rename the test function to
test_redeem_302_sets_no_referrer_policy (and update any references/usages in
test suites or CI filters) so the name accurately reflects the behavior tested
in the redeem handler; locate the function definition in
tests/routes/desktop_browser/test_browser_proxy_origin.py and change only the
function name and any imports/markers that reference it.
tinyagentos/browser_proxy_origin.py (2)

74-104: ⚖️ Poor tradeoff

Expired sessions are not proactively cleaned up.

_session_store grows with each new session and only removes entries on access when expired. Long-running servers with many users could accumulate stale entries. Consider adding periodic cleanup or an LRU eviction strategy.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tinyagentos/browser_proxy_origin.py` around lines 74 - 104, The session store
only prunes expired entries on lookup, so _session_store can grow unbounded; add
a proactive cleanup by implementing a _cleanup_sessions(state) helper that
iterates _session_store(state) and removes entries whose "expires_at" <
time.monotonic(), then call _cleanup_sessions from _session_store (and/or
_new_browser_session) to prune stale entries on each access; optionally enforce
a max size (LRU style: evict oldest by expires_at when len exceeds threshold)
inside the same helper to prevent unbounded growth.

264-264: 💤 Low value

Parameter next shadows Python builtin.

Consider renaming to next_url for clarity, though this has no functional impact.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tinyagentos/browser_proxy_origin.py` at line 264, The parameter named next in
browser_proxy_origin.py (the Query(...) parameter at the function handling the
on-origin proxy path) shadows the Python builtin; rename the parameter to
next_url in the function signature and all internal references, and preserve
existing external query name by passing alias="next" to the Query(...) if you
need backward-compatible request names; update any callers, tests, and doc
references to use next_url internally.
tinyagentos/routes/desktop_browser/proxy_ticket_routes.py (1)

34-38: 💤 Low value

Potential race condition during signing key initialization.

If two requests arrive simultaneously before the key is initialized, both may call os.urandom(32) and one key will overwrite the other. A ticket minted with the "losing" key won't validate. This is a narrow window at startup and self-recovers on retry, but could be avoided with a lock or by initializing the key in the app lifespan.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tinyagentos/routes/desktop_browser/proxy_ticket_routes.py` around lines 34 -
38, The current lazy init of request.app.state.browser_proxy_signing_key using
os.urandom(32) can race when two requests concurrently hit the check; protect it
by introducing a lock stored on app.state (e.g., browser_proxy_signing_key_lock)
and wrap the check-and-set in an async/sync lock acquisition so only one
initializer runs, or alternatively move initialization into the application
lifespan startup so the key is created once at boot; update the code in
proxy_ticket_routes.py to acquire
request.app.state.browser_proxy_signing_key_lock before reading/setting
request.app.state.browser_proxy_signing_key (use asyncio.Lock for async
handlers) to eliminate the race.
desktop/src/lib/__tests__/sw-push-handlers.test.ts (1)

209-210: 💤 Low value

Remove the no-op spies.

vi.spyOn({ subscribePush }, "subscribePush") spies a property on a throwaway object literal, so it neither intercepts the module nor is asserted — the real spies are set up below at Lines 213-220. These two lines have no effect and could mislead a future reader into thinking subscribePush/getVapidPublicKey are already being tracked here.

♻️ Proposed cleanup
-    vi.spyOn({ subscribePush }, "subscribePush");
-    vi.spyOn({ getVapidPublicKey }, "getVapidPublicKey");
-
     // Mock the API calls
     const subscribeSpy = vi.spyOn(
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@desktop/src/lib/__tests__/sw-push-handlers.test.ts` around lines 209 - 210,
Remove the two no-op spies that create throwaway object literals — specifically
delete the lines calling vi.spyOn({ subscribePush }, "subscribePush") and
vi.spyOn({ getVapidPublicKey }, "getVapidPublicKey"); the real spies for
subscribePush and getVapidPublicKey are already set up later (see the existing
spy setup around subscribePush and getVapidPublicKey) so simply remove these
misleading, ineffective calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@desktop/src/apps/BrowserApp/TabRenderer.tsx`:
- Around line 172-188: The cleanup currently calls
getBrowserProxyOrigin().then(...) asynchronously which can race with a new
mount; capture proxyOrigin synchronously in the effect scope (call await/get
synchronously before registering the cleanup) or guard the then callback with
the cancelled flag so the old cleanup bail outs; specifically, in the effect
that defines cancelled, handles, pinnedAgentIdsForEffect and iframe, obtain
proxyOrigin once (via getBrowserProxyOrigin()) and reference that captured
proxyOrigin inside the cleanup instead of calling getBrowserProxyOrigin()
asynchronously, or at minimum check if cancelled is true at the start of the
.then(...) callback and return early to avoid sending stale
iframe.contentWindow.postMessage close messages.

In `@desktop/src/lib/__tests__/sw-push-handlers.test.ts`:
- Around line 209-210: Remove the two no-op spies that create throwaway object
literals — specifically delete the lines calling vi.spyOn({ subscribePush },
"subscribePush") and vi.spyOn({ getVapidPublicKey }, "getVapidPublicKey"); the
real spies for subscribePush and getVapidPublicKey are already set up later (see
the existing spy setup around subscribePush and getVapidPublicKey) so simply
remove these misleading, ineffective calls.

In `@tests/routes/desktop_browser/test_browser_proxy_origin.py`:
- Around line 255-271: The test name test_redeem_302_carries_no_referrer_policy
is misleading because it suggests the header is absent while the assertion
checks the header equals "no-referrer"; rename the test function to
test_redeem_302_sets_no_referrer_policy (and update any references/usages in
test suites or CI filters) so the name accurately reflects the behavior tested
in the redeem handler; locate the function definition in
tests/routes/desktop_browser/test_browser_proxy_origin.py and change only the
function name and any imports/markers that reference it.

In `@tinyagentos/browser_proxy_origin.py`:
- Around line 74-104: The session store only prunes expired entries on lookup,
so _session_store can grow unbounded; add a proactive cleanup by implementing a
_cleanup_sessions(state) helper that iterates _session_store(state) and removes
entries whose "expires_at" < time.monotonic(), then call _cleanup_sessions from
_session_store (and/or _new_browser_session) to prune stale entries on each
access; optionally enforce a max size (LRU style: evict oldest by expires_at
when len exceeds threshold) inside the same helper to prevent unbounded growth.
- Line 264: The parameter named next in browser_proxy_origin.py (the Query(...)
parameter at the function handling the on-origin proxy path) shadows the Python
builtin; rename the parameter to next_url in the function signature and all
internal references, and preserve existing external query name by passing
alias="next" to the Query(...) if you need backward-compatible request names;
update any callers, tests, and doc references to use next_url internally.

In `@tinyagentos/routes/desktop_browser/proxy_ticket_routes.py`:
- Around line 34-38: The current lazy init of
request.app.state.browser_proxy_signing_key using os.urandom(32) can race when
two requests concurrently hit the check; protect it by introducing a lock stored
on app.state (e.g., browser_proxy_signing_key_lock) and wrap the check-and-set
in an async/sync lock acquisition so only one initializer runs, or alternatively
move initialization into the application lifespan startup so the key is created
once at boot; update the code in proxy_ticket_routes.py to acquire
request.app.state.browser_proxy_signing_key_lock before reading/setting
request.app.state.browser_proxy_signing_key (use asyncio.Lock for async
handlers) to eliminate the race.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e283afa4-9888-42da-a8bf-01c671acc35d

📥 Commits

Reviewing files that changed from the base of the PR and between 4658bd6 and e20e8e2.

📒 Files selected for processing (25)
  • data/config.yaml.example
  • desktop/src/apps/BrowserApp/BrowserApp.tsx
  • desktop/src/apps/BrowserApp/TabRenderer.test.tsx
  • desktop/src/apps/BrowserApp/TabRenderer.tsx
  • desktop/src/lib/__tests__/sw-push-handlers.test.ts
  • desktop/src/lib/browser-proxy-config.test.ts
  • desktop/src/lib/browser-proxy-config.ts
  • desktop/src/sw.ts
  • desktop/tsconfig.tsbuildinfo
  • scripts/install-server.sh
  • tests/routes/desktop_browser/test_browser_proxy_origin.py
  • tests/routes/desktop_browser/test_copilot_js.py
  • tests/routes/desktop_browser/test_injector.py
  • tests/routes/desktop_browser/test_sw_asset.py
  • tinyagentos/__main__.py
  • tinyagentos/auth_middleware.py
  • tinyagentos/browser_proxy_origin.py
  • tinyagentos/config.py
  • tinyagentos/routes/desktop_browser/__init__.py
  • tinyagentos/routes/desktop_browser/copilot.js
  • tinyagentos/routes/desktop_browser/injector.py
  • tinyagentos/routes/desktop_browser/proxy.py
  • tinyagentos/routes/desktop_browser/proxy_ticket.py
  • tinyagentos/routes/desktop_browser/proxy_ticket_routes.py
  • tinyagentos/routes/desktop_browser/sw.js

jaylfc added 3 commits May 31, 2026 19:55
…nt (test_main_entry)

Tests for host/port env-var resolution use a bare object() mock for the app.
The new dual-port path's default proxy_port (6970) is non-zero and doesn't
equal the main port, so the code reaches _serve_dual_port and crashes on
app.state before uvicorn.run is ever called. Fix by:

1. Setting TAOS_BROWSER_PROXY_PORT=0 in both tests to force single-port mode
   (the tests exercise host/port resolution only, not dual-port behaviour).
2. Guarding both app.state.browser_proxy_port assignments in __main__ with
   hasattr(app, "state") so real degraded/non-Starlette apps are resilient.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tinyagentos/__main__.py`:
- Around line 63-64: The startup currently only guards setting
app.state.browser_proxy_port but still enters the dual-port path and later
dereferences app.state in _serve_dual_port(), causing a crash; update the
startup flow so the dual-port branch is conditional on hasattr(app, "state") (or
otherwise ensure app.state exists) before calling _serve_dual_port() and before
any dereference of app.state (e.g., protect the call to _serve_dual_port() and
any access at startup that uses app.state.browser_proxy_port), and if app.state
is missing either fall back to the single-port startup path or raise a clear
error and abort startup.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7681bb22-4372-467a-800e-6dec62f86248

📥 Commits

Reviewing files that changed from the base of the PR and between e20e8e2 and 2f2a954.

📒 Files selected for processing (3)
  • scripts/install-server.sh
  • tests/test_main_entry.py
  • tinyagentos/__main__.py
✅ Files skipped from review due to trivial changes (1)
  • tests/test_main_entry.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/install-server.sh

Comment thread tinyagentos/__main__.py
Comment on lines +63 to +64
if hasattr(app, "state"):
app.state.browser_proxy_port = proxy_port
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard the dual-port branch, not just the assignment.

If app lacks state, Line 66 still calls _serve_dual_port(), and Line 87 immediately dereferences app.state, so startup still crashes in dual-port mode. Either fall back to single-port here or fail fast before calling _serve_dual_port().

Suggested fix
-    if hasattr(app, "state"):
-        app.state.browser_proxy_port = proxy_port
-
-    _serve_dual_port(app, host=host, port=port, proxy_port=proxy_port)
+    if not hasattr(app, "state"):
+        import uvicorn
+
+        uvicorn.run(app, host=host, port=port, backlog=128)
+        return
+
+    app.state.browser_proxy_port = proxy_port
+    _serve_dual_port(app, host=host, port=port, proxy_port=proxy_port)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tinyagentos/__main__.py` around lines 63 - 64, The startup currently only
guards setting app.state.browser_proxy_port but still enters the dual-port path
and later dereferences app.state in _serve_dual_port(), causing a crash; update
the startup flow so the dual-port branch is conditional on hasattr(app, "state")
(or otherwise ensure app.state exists) before calling _serve_dual_port() and
before any dereference of app.state (e.g., protect the call to
_serve_dual_port() and any access at startup that uses
app.state.browser_proxy_port), and if app.state is missing either fall back to
the single-port startup path or raise a clear error and abort startup.

@jaylfc jaylfc merged commit 30085c5 into master May 31, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in TinyAgentOS Roadmap May 31, 2026
@jaylfc jaylfc deleted the feat/browser-proxy-origin branch May 31, 2026 20:28
jaylfc added a commit that referenced this pull request Jun 2, 2026
… cross-origin CSP) (#518)

* fix(browser): make the proxy work on real deploys (dual-port launch + cross-origin CSP)

The Browser app (#511) never worked on a systemd deploy. Two bugs:

1. The unit's ExecStart ran 'uvicorn tinyagentos.app:create_app' directly,
   which never starts the second browser-proxy origin (that only launches
   via __main__.main() -> _serve_dual_port). So nothing listened on the
   proxy port and pages failed to load. Switch ExecStart to
   'python -m tinyagentos' and inject TAOS_HOST/TAOS_PORT into the unit
   Environment (install-server.sh) so the module entrypoint binds the same
   host/port and runs both origins.

2. Proxied responses set 'frame-ancestors self', which blocks the taOS
   shell (main origin) from embedding the proxy origin (different port =
   different origin) — so even with both origins up, the iframe was CSP-
   blocked. Compute frame-ancestors per-request as "'self' <shell-origin>"
   (same host, main port), so only the shell can frame it (clickjacking
   defence preserved). Requires main_port on app.state and main_port/
   browser_proxy_port added to the proxy origin's shared-state allowlist.

Verified live: example.org renders through the proxy and links are
rewritten to flow back through it. Adds CSP unit tests for both the
single-port ('self' only) and dual-origin (shell origin allowed) cases.

* fix(deploy): graceful-stop hook honours TAOS_PORT instead of hardcoding 6969

Addresses CodeRabbit on #517: a custom-port install would drain a
hardcoded localhost:6969 (silently no-op via '|| true'). systemd already
passes TAOS_PORT into the unit Environment (this PR), so the hook inherits
it and falls back to 6969 only when unset.

* fix(browser): robust host/scheme parsing in _shell_origin (CodeRabbit #518)

- Handle IPv6 literal Host headers: a bare [::1] would be mangled by
  rsplit(':',1) into '[', producing a malformed frame-ancestors CSP that
  breaks framing for IPv6 access. Extract via the closing bracket instead.
- Clamp x-forwarded-proto to http/https (take the first value of a comma
  list) so a hostile/odd proto can't deform the CSP.
- Unit tests for _strip_port covering host:port, bare host, IPv6 ±port, empty.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant