fix(browser): make the proxy work on real deploys (dual-port launch + cross-origin CSP)#518
Merged
Merged
Conversation
… 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.
|
Warning Review limit reached
More reviews will be available in 11 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe pull request refactors TinyAgentOS startup to use a module entrypoint and enables the desktop browser proxy to compute Content-Security-Policy ChangesDynamic Frame-Ancestors CSP
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…ng 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@scripts/install-server.sh`:
- Around line 1167-1170: The current in-place sed invocation that injects
Environment lines into the user unit (the sed -i ... on "$unit") is
Linux-specific and inconsistent with the earlier temp-file render approach;
replace this sed -i usage with the same temp-file/atomic-rewrite pattern used
for the initial unit rendering: create a temporary file, write the existing file
contents but with the Environment=PYTHONUNBUFFERED=1 line followed by the three
Environment=TAOS_* lines (expanding $TAOS_PORT and $TAOS_BROWSER_PROXY_PORT),
then mv the temp file over "$unit" (or use an atomic install method) so the
operation is portable and consistent with the initial unit rendering logic.
- Around line 1122-1125: The use of sed -i with embedded "\n" is not portable;
replace the single-line sed -i invocation that injects Environment lines into
"$unit" with a POSIX-safe temp-file approach: run a sed command that uses the
append (a\) syntax to add the three Environment= lines after the matching
Environment=PYTHONUNBUFFERED=1 line (escaping newlines per sed's a\ form) and
write the output to a temp file, then use $sudo_cmd install (same pattern used
earlier) to atomically install the temp file over "$unit"; ensure you reference
TAOS_PORT and TAOS_BROWSER_PROXY_PORT when emitting the Environment lines and
remove the sed -i usage entirely.
In `@tinyagentos/routes/desktop_browser/proxy.py`:
- Around line 112-132: The _shell_origin function incorrectly strips ports from
IPv6 Host headers and trusts x-forwarded-proto; fix by robustly parsing the Host
header and validating the scheme: in _shell_origin, derive host by checking if
the Host header starts with '[' (take up to the matching ']') else split on the
last ':' to remove a numeric port, ensure the resulting host is non-empty, and
validate scheme = request.headers.get("x-forwarded-proto") or request.url.scheme
or "http" against an allowed set (e.g., "http" and "https") falling back to
"http" on invalid values; return f"{scheme}://{host}:{main_port}" only when host
and scheme pass validation.
🪄 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: ef9c4898-0516-44da-93c1-a980e326df41
📒 Files selected for processing (7)
scripts/install-server.shscripts/systemd/tinyagentos.servicetests/routes/desktop_browser/test_csp.pytinyagentos/__main__.pytinyagentos/browser_proxy_origin.pytinyagentos/routes/desktop_browser/csp.pytinyagentos/routes/desktop_browser/proxy.py
Comment on lines
+1122
to
+1125
| # Inject bind host/port + proxy port into the unit's Environment block. | ||
| # ExecStart now runs `python -m tinyagentos`, which reads these (rather | ||
| # than uvicorn CLI args), so the dual-port browser-proxy origin starts. | ||
| $sudo_cmd sed -i "s|^Environment=PYTHONUNBUFFERED=1|Environment=PYTHONUNBUFFERED=1\nEnvironment=TAOS_HOST=0.0.0.0\nEnvironment=TAOS_PORT=$TAOS_PORT\nEnvironment=TAOS_BROWSER_PROXY_PORT=$TAOS_BROWSER_PROXY_PORT|" "$unit" |
There was a problem hiding this comment.
sed -i with \n may not be portable across Linux and macOS.
The script uses sed -i with \n in the replacement string to inject multiple Environment= lines. This has two portability issues:
sed -isyntax differs: macOS BSD sed requiressed -i '', while GNU sed (Linux) usessed -i. The script earlier (lines 1113-1121) avoided this by writing to a temp file first, then usinginstall.\nin replacement: GNU sed interprets\nas a literal newline in the replacement, but BSD sed may not. Some implementations require an actual embedded newline.
Both issues would cause the systemd unit to be malformed or the sed command to fail on macOS (though this is a Linux-only code path, so macOS is not affected here).
🔧 Proposed fix: Use consistent temp-file pattern
- # Inject bind host/port + proxy port into the unit's Environment block.
- # ExecStart now runs `python -m tinyagentos`, which reads these (rather
- # than uvicorn CLI args), so the dual-port browser-proxy origin starts.
- $sudo_cmd sed -i "s|^Environment=PYTHONUNBUFFERED=1|Environment=PYTHONUNBUFFERED=1\nEnvironment=TAOS_HOST=0.0.0.0\nEnvironment=TAOS_PORT=$TAOS_PORT\nEnvironment=TAOS_BROWSER_PROXY_PORT=$TAOS_BROWSER_PROXY_PORT|" "$unit"
+ # Inject bind host/port + proxy port into the unit's Environment block.
+ # ExecStart now runs `python -m tinyagentos`, which reads these (rather
+ # than uvicorn CLI args), so the dual-port browser-proxy origin starts.
+ $sudo_cmd sed \
+ -e "/^Environment=PYTHONUNBUFFERED=1/a\\
+Environment=TAOS_HOST=0.0.0.0\\
+Environment=TAOS_PORT=$TAOS_PORT\\
+Environment=TAOS_BROWSER_PROXY_PORT=$TAOS_BROWSER_PROXY_PORT" \
+ "$unit" > /tmp/tinyagentos.service.postenv
+ $sudo_cmd mv /tmp/tinyagentos.service.postenv "$unit"This uses a\ (append) command with backslash-escaped newlines, which is POSIX-compliant, and avoids sed -i.
🤖 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 `@scripts/install-server.sh` around lines 1122 - 1125, The use of sed -i with
embedded "\n" is not portable; replace the single-line sed -i invocation that
injects Environment lines into "$unit" with a POSIX-safe temp-file approach: run
a sed command that uses the append (a\) syntax to add the three Environment=
lines after the matching Environment=PYTHONUNBUFFERED=1 line (escaping newlines
per sed's a\ form) and write the output to a temp file, then use $sudo_cmd
install (same pattern used earlier) to atomically install the temp file over
"$unit"; ensure you reference TAOS_PORT and TAOS_BROWSER_PROXY_PORT when
emitting the Environment lines and remove the sed -i usage entirely.
Comment on lines
+1167
to
+1170
| # Inject bind host/port + proxy port into the unit's Environment block. | ||
| # ExecStart now runs `python -m tinyagentos`, which reads these (rather | ||
| # than uvicorn CLI args), so the dual-port browser-proxy origin starts. | ||
| sed -i "s|^Environment=PYTHONUNBUFFERED=1|Environment=PYTHONUNBUFFERED=1\nEnvironment=TAOS_HOST=0.0.0.0\nEnvironment=TAOS_PORT=$TAOS_PORT\nEnvironment=TAOS_BROWSER_PROXY_PORT=$TAOS_BROWSER_PROXY_PORT|" "$unit" |
There was a problem hiding this comment.
Same sed -i portability issue in user unit path.
The user unit installation uses the same sed -i with \n pattern. While this is Linux-only (macOS uses launchd), the approach is inconsistent with the earlier temp-file pattern used for the initial unit rendering (lines 1154-1166).
🔧 Proposed fix: Use consistent append pattern
- # Inject bind host/port + proxy port into the unit's Environment block.
- # ExecStart now runs `python -m tinyagentos`, which reads these (rather
- # than uvicorn CLI args), so the dual-port browser-proxy origin starts.
- sed -i "s|^Environment=PYTHONUNBUFFERED=1|Environment=PYTHONUNBUFFERED=1\nEnvironment=TAOS_HOST=0.0.0.0\nEnvironment=TAOS_PORT=$TAOS_PORT\nEnvironment=TAOS_BROWSER_PROXY_PORT=$TAOS_BROWSER_PROXY_PORT|" "$unit"
+ # Inject bind host/port + proxy port into the unit's Environment block.
+ # ExecStart now runs `python -m tinyagentos`, which reads these (rather
+ # than uvicorn CLI args), so the dual-port browser-proxy origin starts.
+ sed \
+ -e "/^Environment=PYTHONUNBUFFERED=1/a\\
+Environment=TAOS_HOST=0.0.0.0\\
+Environment=TAOS_PORT=$TAOS_PORT\\
+Environment=TAOS_BROWSER_PROXY_PORT=$TAOS_BROWSER_PROXY_PORT" \
+ "$unit" > "$unit.tmp"
+ mv "$unit.tmp" "$unit"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Inject bind host/port + proxy port into the unit's Environment block. | |
| # ExecStart now runs `python -m tinyagentos`, which reads these (rather | |
| # than uvicorn CLI args), so the dual-port browser-proxy origin starts. | |
| sed -i "s|^Environment=PYTHONUNBUFFERED=1|Environment=PYTHONUNBUFFERED=1\nEnvironment=TAOS_HOST=0.0.0.0\nEnvironment=TAOS_PORT=$TAOS_PORT\nEnvironment=TAOS_BROWSER_PROXY_PORT=$TAOS_BROWSER_PROXY_PORT|" "$unit" | |
| # Inject bind host/port + proxy port into the unit's Environment block. | |
| # ExecStart now runs `python -m tinyagentos`, which reads these (rather | |
| # than uvicorn CLI args), so the dual-port browser-proxy origin starts. | |
| sed \ | |
| -e "/^Environment=PYTHONUNBUFFERED=1/a\\ | |
| Environment=TAOS_HOST=0.0.0.0\\ | |
| Environment=TAOS_PORT=$TAOS_PORT\\ | |
| Environment=TAOS_BROWSER_PROXY_PORT=$TAOS_BROWSER_PROXY_PORT" \ | |
| "$unit" > "$unit.tmp" | |
| mv "$unit.tmp" "$unit" |
🤖 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 `@scripts/install-server.sh` around lines 1167 - 1170, The current in-place sed
invocation that injects Environment lines into the user unit (the sed -i ... on
"$unit") is Linux-specific and inconsistent with the earlier temp-file render
approach; replace this sed -i usage with the same temp-file/atomic-rewrite
pattern used for the initial unit rendering: create a temporary file, write the
existing file contents but with the Environment=PYTHONUNBUFFERED=1 line followed
by the three Environment=TAOS_* lines (expanding $TAOS_PORT and
$TAOS_BROWSER_PROXY_PORT), then mv the temp file over "$unit" (or use an atomic
install method) so the operation is portable and consistent with the initial
unit rendering logic.
…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.
This was referenced Jun 2, 2026
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
1 participant
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.
Problem
The Browser app (#511) does not work on a real systemd deploy — pages fail to load. Root-caused live on a Pi. Two independent bugs:
1. The second proxy origin never starts
The systemd unit ran
uvicorn tinyagentos.app:create_appdirectly. The dual-port browser-proxy origin only launches via__main__.main()→_serve_dual_port, so nothing listened on the proxy port (proxy-configreturned{"port":0}) and the iframe's redeem→proxy chain 404'd.Fix:
ExecStart→python -m tinyagentos, and injectTAOS_HOST/TAOS_PORTinto the unit Environment (ininstall-server.sh, both system + user paths) so the module entrypoint binds the same host/port and runs both origins.TAOS_BROWSER_PROXY_PORTwas already injected.2. Cross-origin framing was CSP-blocked
Proxied responses set
frame-ancestors 'self'. The shell (main port) and proxy (proxy port) are different origins, so this blocked the shell from embedding the proxy iframe entirely — even once both origins were up.Fix: compute
frame-ancestorsper-request as'self' <shell-origin>(same host, main port), so only the taOS shell can frame the proxy (clickjacking defence preserved). Needsmain_portonapp.stateandmain_port/browser_proxy_portadded to the proxy origin's_SharedStateallowlist.Verification
Verified live on a Pi after deploying these files: the Browser app renders real pages through the proxy and links are rewritten to flow back through it (was "Not Found" / CSP-blocked before).
Tests
frame-ancestorsis'self'alone in single-port fallback, and'self' <shell-origin>when a shell origin is supplied.Summary by CodeRabbit
New Features
Tests