[auto] #82 feat: expose tt serve beyond localhost for remote agent access - #88
Merged
Conversation
…mote agent access
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds remote serving capabilities to the Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant Serve as Serve Handler
participant Config as Config System
participant Auth as Auth & Token
participant Server as HTTP Server
participant Client as HTTP Client
User->>Serve: tt serve --remote --bind 0.0.0.0
Serve->>Config: Load GlobalConfig
Config-->>Serve: Config with serve settings
Serve->>Serve: Resolve bind address<br/>(precedence: --bind → --remote → config → 127.0.0.1)
Serve->>Serve: Determine auth mode<br/>(remote → Bearer, else config)
Serve->>Serve: Validate: no auth + non-localhost<br/>= reject
alt Bearer Auth Enabled
Serve->>Auth: load_or_generate_serve_token()
Auth->>Auth: Check ~/.config/tutti/serve-token
alt Token Exists
Auth-->>Serve: Load existing token
else Token Missing
Auth->>Auth: Generate 256-bit random hex
Auth->>Auth: Write to disk (0600 perms)
Auth-->>Serve: Return new token
end
Serve->>Serve: Print bearer token
end
Serve->>Server: start_control_http_server(bind_addr, token?)
Client->>Server: HTTP request
Server->>Server: Check Authorization header<br/>for Bearer token
alt Valid Token
Server-->>Client: 200 OK + response
else Invalid/Missing Token
Server-->>Client: 401 Unauthorized (JSON)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
… parsing Extract validate_bearer_auth and is_localhost_addr into testable helpers. Add 8 unit tests covering bearer auth validation, token generation/reload, non-localhost detection, and ServeConfig TOML round-trip deserialization. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/config/mod.rs (1)
333-354: Add focused tests for[serve]deserialization defaults and auth parsing.Please add unit tests covering: missing
[serve](serve = None), empty[serve](default bind/auth), and explicitauth = "bearer". This hardens a security-sensitive config path against regressions.As per coding guidelines "**/*.rs: Write unit tests in each module using
#[cfg(test)] mod tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/mod.rs` around lines 333 - 354, Add a #[cfg(test)] mod tests in this module and add unit tests that deserialize TOML into a small test wrapper to validate defaults and parsing: create a local #[derive(Deserialize)] struct Root { serve: Option<ServeConfig> } and assert that deserializing TOML with no [serve] yields Root.serve == None; deserializing with an empty table ([serve] or serve = {}) yields Some(ServeConfig) with bind == default_serve_bind() and auth == ServeAuthMode::None; and deserializing with auth = "bearer" yields ServeAuthMode::Bearer. Use toml::from_str in each test and assert equality against ServeConfig/ServeAuthMode to ensure defaults and auth parsing work.src/cli/serve.rs (3)
360-368: Consider using constant-time comparison for token validation.The string comparison
t == tokenis not constant-time, which could theoretically leak timing information. While the risk is low for this use case, using a constant-time comparison would follow security best practices.🔒 Suggested improvement using subtle crate
+use subtle::ConstantTimeEq; + fn validate_bearer_auth(auth_header: Option<&str>, expected: Option<&str>) -> bool { let Some(token) = expected else { return true; }; auth_header .and_then(|v| v.strip_prefix("Bearer ")) - .map(|t| t == token) + .map(|t| t.as_bytes().ct_eq(token.as_bytes()).into()) .unwrap_or(false) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/serve.rs` around lines 360 - 368, The current equality check in validate_bearer_auth uses `t == token`, which is not constant-time; replace it with a constant-time byte comparison (e.g., using subtle::ConstantTimeEq or another constant-time compare) by converting both strings to bytes, checking lengths equal, then using ct_eq on the slices and converting the result into a bool; keep the same function signature and flow (validate_bearer_auth, auth_header strip_prefix "Bearer ") but return the constant-time comparison result instead of `t == token`.
1286-1289: Consider reusing config directory helper and improving HOME fallback.Two concerns with the path construction:
Path duplication: Per the relevant code snippet,
src/config/mod.rshasglobal_config_path()that usesdirs_or_home().join(".config").join("tutti"). Consider extracting a sharedtutti_config_dir()helper to avoid maintenance risk if the config root becomes configurable.Unsafe fallback: Falling back to
"."when HOME is unset would write the token to the current directory, which is likely unintended and could expose the token.♻️ Suggested improvement
fn load_or_generate_serve_token() -> Result<String> { - let home = std::env::var("HOME") - .map(PathBuf::from) - .unwrap_or_else(|_| PathBuf::from(".")); - let config_dir = home.join(".config").join("tutti"); + let config_dir = crate::config::global_config_path() + .parent() + .ok_or_else(|| TuttiError::ConfigValidation( + "could not determine config directory".to_string() + ))? + .to_path_buf(); let token_path = config_dir.join("serve-token");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/serve.rs` around lines 1286 - 1289, Replace the ad-hoc HOME lookup in serve.rs with a shared config-dir helper: either call the existing config::global_config_path() or add a new config::tutti_config_dir() wrapper around dirs_or_home().join(".config").join("tutti"), and use that value for config_dir; also stop falling back to PathBuf::from(".")—use dirs_or_home() or dirs::home_dir() (the same robust fallback used by dirs_or_home()) so we never write tokens to the current working directory and centralize path logic in the config module (update imports to reference global_config_path or the new tutti_config_dir helper).
1490-1516: Environment variable modification may cause test flakiness.The test modifies the
HOMEenvironment variable, which is shared process-wide state. If tests run in parallel (default forcargo test), this could cause race conditions with other tests that depend onHOME.Consider using the
serial_testcrate to mark this test as#[serial], or restructureload_or_generate_serve_tokento accept a config directory path for testability.♻️ Alternative approach: make function testable via dependency injection
+fn load_or_generate_serve_token() -> Result<String> { + let config_dir = crate::config::global_config_path() + .parent() + .ok_or_else(|| TuttiError::ConfigValidation( + "could not determine config directory".to_string() + ))? + .to_path_buf(); + load_or_generate_serve_token_in(&config_dir) +} + +fn load_or_generate_serve_token_in(config_dir: &Path) -> Result<String> { let token_path = config_dir.join("serve-token"); // ... rest of implementation +} // In tests: #[test] fn token_generation_produces_valid_hex_and_reloads() { let temp = tempfile::tempdir().unwrap(); let token1 = load_or_generate_serve_token_in(temp.path()).unwrap(); // ... assertions }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/serve.rs` around lines 1490 - 1516, The test token_generation_produces_valid_hex_and_reloads mutates the process-wide HOME env var causing flakiness in parallel runs; fix by either annotating the test with #[serial] (using the serial_test crate) to force exclusive execution, or refactor load_or_generate_serve_token to accept an optional config_dir/path argument (and update the test to call the new function with a temp dir) so the test no longer needs to modify HOME; reference the test name token_generation_produces_valid_hex_and_reloads and the function load_or_generate_serve_token when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/serve.rs`:
- Around line 1283-1285: The doc comment currently intended for
load_or_generate_serve_token is attached to resolve_default_port; fix it by
moving the triple-slash comment block so it sits immediately above the
load_or_generate_serve_token() signature (ensure there is a blank line or
separator after resolve_default_port to prevent accidental attachment), i.e.,
remove the misplaced /// block from before resolve_default_port and place it
directly above fn load_or_generate_serve_token() so the documentation binds to
the correct function.
- Around line 1301-1306: The token generation currently reads /dev/urandom
(bytes, reader, token) which is not cross-platform; replace that block with
getrandom::getrandom to fill the 32-byte array (the same `let mut bytes = [0u8;
32];`) and convert any getrandom error into the existing TuttI/O error type (use
map_err to map getrandom::Error to TuttError::Io). Keep the subsequent hex
encoding step (bytes.iter().map(...).collect()) unchanged so `token` remains the
same format.
---
Nitpick comments:
In `@src/cli/serve.rs`:
- Around line 360-368: The current equality check in validate_bearer_auth uses
`t == token`, which is not constant-time; replace it with a constant-time byte
comparison (e.g., using subtle::ConstantTimeEq or another constant-time compare)
by converting both strings to bytes, checking lengths equal, then using ct_eq on
the slices and converting the result into a bool; keep the same function
signature and flow (validate_bearer_auth, auth_header strip_prefix "Bearer ")
but return the constant-time comparison result instead of `t == token`.
- Around line 1286-1289: Replace the ad-hoc HOME lookup in serve.rs with a
shared config-dir helper: either call the existing config::global_config_path()
or add a new config::tutti_config_dir() wrapper around
dirs_or_home().join(".config").join("tutti"), and use that value for config_dir;
also stop falling back to PathBuf::from(".")—use dirs_or_home() or
dirs::home_dir() (the same robust fallback used by dirs_or_home()) so we never
write tokens to the current working directory and centralize path logic in the
config module (update imports to reference global_config_path or the new
tutti_config_dir helper).
- Around line 1490-1516: The test
token_generation_produces_valid_hex_and_reloads mutates the process-wide HOME
env var causing flakiness in parallel runs; fix by either annotating the test
with #[serial] (using the serial_test crate) to force exclusive execution, or
refactor load_or_generate_serve_token to accept an optional config_dir/path
argument (and update the test to call the new function with a temp dir) so the
test no longer needs to modify HOME; reference the test name
token_generation_produces_valid_hex_and_reloads and the function
load_or_generate_serve_token when making the change.
In `@src/config/mod.rs`:
- Around line 333-354: Add a #[cfg(test)] mod tests in this module and add unit
tests that deserialize TOML into a small test wrapper to validate defaults and
parsing: create a local #[derive(Deserialize)] struct Root { serve:
Option<ServeConfig> } and assert that deserializing TOML with no [serve] yields
Root.serve == None; deserializing with an empty table ([serve] or serve = {})
yields Some(ServeConfig) with bind == default_serve_bind() and auth ==
ServeAuthMode::None; and deserializing with auth = "bearer" yields
ServeAuthMode::Bearer. Use toml::from_str in each test and assert equality
against ServeConfig/ServeAuthMode to ensure defaults and auth parsing work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c5860fd9-4e68-44c7-ad0a-24d7b1fd136e
📒 Files selected for processing (8)
src/cli/doctor.rssrc/cli/mod.rssrc/cli/serve.rssrc/cli/up.rssrc/cli/usage.rssrc/cli/watch.rssrc/config/mod.rssrc/main.rs
- Replace /dev/urandom with getrandom crate for cross-platform support - Remove misplaced doc comment line from load_or_generate_serve_token Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 20, 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.
Summary
feat: expose tt serve beyond localhost for remote agent accessVersioning (required)
Cargo.tomlversionCHANGELOG.mdSemVer choice
Version selected:
v0.0.0Validation
cargo test -qRelease
vX.Y.Z)Summary by CodeRabbit
New Features
--remoteflag to bind the serve command to all interfaces for remote access.--bindoption to specify a custom bind address for the serve command.~/.config/tutti/config.toml).Tests