feat(core): add authenticated static directory hosting#1966
Merged
senamakel merged 3 commits intoMay 16, 2026
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an http_host subsystem providing types, ops, RPC adapters, controller schemas, request routing, Basic Auth, path-safety, tests, and global registration to start/list/get/stop in-process static-directory HTTP servers. ChangesHTTP Hosted Directory Server
Sequence Diagram(s)sequenceDiagram
participant Caller
participant RPC as http_host::rpc
participant Ops as http_host::ops
participant Registry
participant Axum
participant FS as Filesystem
Caller->>RPC: start(StartHostedDirParams)
RPC->>Ops: start_hosted_dir_server(params)
Ops->>Registry: check/register ServerRuntime
Ops->>Axum: spawn server task (with HostedDirState)
Ops-->>Caller: HostedDirStartResult
Note over Axum: Incoming HTTP request
Axum->>Axum: ensure_authorized(headers, auth)
Axum->>Axum: resolve_request_path(root, path)
Axum->>FS: read file or list directory
Axum-->>Caller: file bytes or HTML listing
Caller->>RPC: stop(HostedDirLookupParams)
RPC->>Ops: stop_hosted_dir_server(server_id)
Ops->>Registry: remove & cancel token
Ops-->>Caller: HostedDirStopResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/http_host/ops.rs (1)
1-787: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit this module; it is well beyond the repository size limit.
This file is 787 lines, which makes maintenance and review harder. Please split by concerns (e.g., registry/lifecycle, auth, path safety, HTTP handlers, tests).
As per coding guidelines:
Rust modules should generally not exceed ~500 lines; split growing modules into smaller units.🤖 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 `@src/openhuman/http_host/ops.rs` around lines 1 - 787, Split this large ops.rs into focused modules: move registry/lifecycle items (HostedDirRegistry, HostedDirRuntime, registry(), register_shutdown_hook_once(), start_hosted_dir_server, stop_hosted_dir_server, stop_all_hosted_dir_servers, list_hosted_dir_servers, get_hosted_dir_server) into a new module (e.g., hosted_dir/registry.rs); move HTTP handlers and runtime state (HostedDirState, serve_root, serve_path, serve_relative_path, serve_file, serve_directory, Router construction) into hosted_dir/handlers.rs; move auth logic (ensure_authorized, unauthorized_response, resolve_default_auth_username, fallback_env_username, sanitize_basic_auth_username, resolve_default_auth_username_from_user_value, generate_password) into hosted_dir/auth.rs; move path safety and util helpers (canonicalize_hosted_directory, sanitize_bind_host, sanitize_optional_label, resolve_request_path, parent_href_for, child_href_for, escape_html, render_host_for_url, content_type_for_path) into hosted_dir/path.rs; update mod declarations and imports to re-export types like HostedDirServerInfo and StartHostedDirParams and adjust uses in tests; finally extract the #[cfg(test)] tests into hosted_dir/tests.rs (or keep under module tests) and use shared test helpers (TEST_MUTEX) as needed. Ensure all moved functions keep the same signatures and update uses of LOG_PREFIX and types so code compiles.
🤖 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 `@src/core/all.rs`:
- Around line 130-131: You register HTTP host controllers via
controllers.extend(crate::openhuman::http_host::all_http_host_registered_controllers())
but there are no corresponding entries added in
build_declared_controller_schemas(), so validate_registry can fail; fix by
adding the http_host controller schemas to build_declared_controller_schemas()
(or have all_http_host_registered_controllers expose its schemas and merge them
there) so that the declared schemas include the http_host handlers before
validate_registry runs (update build_declared_controller_schemas(),
validate_registry usage, or adjust all_http_host_registered_controllers to
return both controllers and schemas).
In `@src/openhuman/http_host/ops.rs`:
- Around line 102-108: The log currently prints the full absolute path via
root_dir.display() (in the log::info call and the later similar logs around the
same area), which can expose PII—replace that usage with a redacted
representation instead (e.g., a helper like redact_path(root_dir) that returns
either the path basename, last N segments, or a masked string such as
"<redacted>" or "~/<last_segment>"); update the log::info calls that reference
root_dir (and any other places logging full directory paths in this module) to
call that redaction helper so logs never contain full directory paths.
- Around line 331-355: serve_file currently reads the whole file into memory
with tokio::fs::read; change it to open the file with tokio::fs::File::open and
stream it to the response using tokio_util::io::ReaderStream (or FramedRead)
wrapped in Body::wrap_stream so the payload is sent as a streaming body. On
error opening the file, log the error (keep the existing log message pattern)
and return the same INTERNAL_SERVER_ERROR response; on success set
StatusCode::OK and the content type header (using content_type_for_path) and
return the Response whose body is Body::wrap_stream(reader_stream) so large
files don't allocate fully in memory.
In `@src/openhuman/http_host/schemas.rs`:
- Around line 93-109: The ControllerSchema for "stop" is missing the "stopped"
output that rpc::stop returns; update the "stop" ControllerSchema (in the block
defining ControllerSchema { namespace: "http_host", function: "stop", ... }) to
include an additional outputs FieldSchema with name: "stopped", ty:
TypeSchema::Bool, comment: something like "Whether the server was successfully
stopped.", and required: true so the schema matches rpc::stop's returned {
stopped, server } shape.
In `@src/openhuman/http_host/types.rs`:
- Around line 36-37: Change the default bind host in the function
default_bind_host() from "0.0.0.0" to the loopback address "127.0.0.1" so the
HTTP Basic-auth endpoint is not exposed to the LAN by default; keep the rest of
the function unchanged and ensure any docs or configuration that mention the
previous default are updated so external binding remains an explicit opt-in.
---
Outside diff comments:
In `@src/openhuman/http_host/ops.rs`:
- Around line 1-787: Split this large ops.rs into focused modules: move
registry/lifecycle items (HostedDirRegistry, HostedDirRuntime, registry(),
register_shutdown_hook_once(), start_hosted_dir_server, stop_hosted_dir_server,
stop_all_hosted_dir_servers, list_hosted_dir_servers, get_hosted_dir_server)
into a new module (e.g., hosted_dir/registry.rs); move HTTP handlers and runtime
state (HostedDirState, serve_root, serve_path, serve_relative_path, serve_file,
serve_directory, Router construction) into hosted_dir/handlers.rs; move auth
logic (ensure_authorized, unauthorized_response, resolve_default_auth_username,
fallback_env_username, sanitize_basic_auth_username,
resolve_default_auth_username_from_user_value, generate_password) into
hosted_dir/auth.rs; move path safety and util helpers
(canonicalize_hosted_directory, sanitize_bind_host, sanitize_optional_label,
resolve_request_path, parent_href_for, child_href_for, escape_html,
render_host_for_url, content_type_for_path) into hosted_dir/path.rs; update mod
declarations and imports to re-export types like HostedDirServerInfo and
StartHostedDirParams and adjust uses in tests; finally extract the #[cfg(test)]
tests into hosted_dir/tests.rs (or keep under module tests) and use shared test
helpers (TEST_MUTEX) as needed. Ensure all moved functions keep the same
signatures and update uses of LOG_PREFIX and types so code compiles.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 066407be-c1cc-4b29-823a-5ac83279db02
📒 Files selected for processing (7)
src/core/all.rssrc/openhuman/http_host/mod.rssrc/openhuman/http_host/ops.rssrc/openhuman/http_host/rpc.rssrc/openhuman/http_host/schemas.rssrc/openhuman/http_host/types.rssrc/openhuman/mod.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@src/core/all.rs`:
- Around line 130-131: The http_host controllers are being added to the
agent-facing catalog via
controllers.extend(crate::openhuman::http_host::all_http_host_registered_controllers()),
which exposes local-directory hosting and Basic Auth to agents; remove this
extend call from the public/all-facing registration and instead register those
controllers only in the internal-only catalog (e.g., inside
build_internal_only_controllers() or whatever internal registration function you
have) so they remain available to RPC/trusted callers but not discoverable to
agents or tool listings. Ensure you update references to
crate::openhuman::http_host::all_http_host_registered_controllers() so it is
invoked only from the internal registration path and not from the public
controllers list.
In `@src/openhuman/http_host/handlers.rs`:
- Around line 65-70: The warning logs currently print absolute filesystem paths
via resolved.display(), which can leak PII; update each log::warn! that logs the
path (the Err branches where you log "metadata failed path=... err=..." and
similar occurrences later in the file) to call redact_path_for_log(&resolved)
instead of resolved.display(), and ensure you pass the redacted string into the
log macro (e.g., log::warn!("{} metadata failed path={} err={}", LOG_PREFIX,
redact_path_for_log(&resolved), error)). Apply the same replacement for the
other two occurrences mentioned (the similar log::warn! blocks around the other
metadata error handlers).
- Around line 117-128: The current directory iteration using while let
Ok(Some(entry)) = entries.next_entry().await will silently stop on the first
read error and return a partial listing; change the loop to explicitly match on
entries.next_entry().await and handle all arms: Ok(Some(entry)) -> process and
push to rows (keeping the existing file_type handling), Ok(None) -> break the
loop, Err(e) -> log the error and return an Err (or propagate an appropriate
HTTP error) so the request fails instead of returning a partial listing; update
the logic around entries, rows, and entry.file_type().await to reflect this
explicit match and error propagation.
In `@src/openhuman/http_host/ops.rs`:
- Around line 257-260: The loop currently cancels and awaits each runtime
sequentially, so a long-running join can delay cancelling other runtimes;
instead, first iterate over runtimes and call runtime.shutdown.cancel() for
every (preserving server_id visibility if needed), then in a separate loop await
each runtime.join_handle.await (and handle errors) so all cancellations are
issued before any await blocks; update the code around the runtimes iteration to
perform cancelation and awaiting in two distinct passes using the existing
runtimes, runtime.shutdown.cancel(), and runtime.join_handle.await symbols.
- Around line 91-101: The bind_target concatenation in ops::start (where
bind_target is built and passed to TcpListener::bind) doesn't bracket IPv6
literals, causing binds like "::1:3000" to fail; update the logic that
constructs bind_target to mirror render_host_for_url's handling of IPv6 (i.e.,
detect a bare IPv6 host containing ':' and not already bracketed and wrap it in
[ ] before appending :{port}), or otherwise produce a valid SocketAddr string
for TcpListener::bind; ensure TcpListener::bind gets the bracketed host:port
form so IPv6 and IPv4 both work.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 103a0bc1-76d4-423b-85e5-02ddc7a3d45f
📒 Files selected for processing (10)
Cargo.tomlsrc/core/all.rssrc/openhuman/http_host/auth.rssrc/openhuman/http_host/handlers.rssrc/openhuman/http_host/mod.rssrc/openhuman/http_host/ops.rssrc/openhuman/http_host/path_utils.rssrc/openhuman/http_host/schemas.rssrc/openhuman/http_host/tests.rssrc/openhuman/http_host/types.rs
✅ Files skipped from review due to trivial changes (1)
- Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- src/openhuman/http_host/mod.rs
- src/openhuman/http_host/types.rs
- src/openhuman/http_host/schemas.rs
Comment on lines
+130
to
+131
| // Ad-hoc static directory HTTP hosting for local file sharing / previews | ||
| controllers.extend(crate::openhuman::http_host::all_http_host_registered_controllers()); |
Contributor
There was a problem hiding this comment.
Keep http_host out of the agent-facing catalog if this is meant for trusted callers only.
Registering it here makes the namespace discoverable to agents/tool listings, not just RPC callers. For a surface that can expose arbitrary local directories and returns the generated Basic Auth credentials, that is a much broader trust boundary than build_internal_only_controllers().
Also applies to: 265-265
🤖 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 `@src/core/all.rs` around lines 130 - 131, The http_host controllers are being
added to the agent-facing catalog via
controllers.extend(crate::openhuman::http_host::all_http_host_registered_controllers()),
which exposes local-directory hosting and Basic Auth to agents; remove this
extend call from the public/all-facing registration and instead register those
controllers only in the internal-only catalog (e.g., inside
build_internal_only_controllers() or whatever internal registration function you
have) so they remain available to RPC/trusted callers but not discoverable to
agents or tool listings. Ensure you update references to
crate::openhuman::http_host::all_http_host_registered_controllers() so it is
invoked only from the internal registration path and not from the public
controllers list.
Comment on lines
+65
to
+70
| Err(error) => { | ||
| log::warn!( | ||
| "{LOG_PREFIX} metadata failed path={} err={}", | ||
| resolved.display(), | ||
| error | ||
| ); |
Contributor
There was a problem hiding this comment.
Redact filesystem paths in warning logs.
These warnings still emit absolute local paths via .display(), which can leak usernames and other local PII. Use redact_path_for_log(...) at these sites too.
As per coding guidelines: Never log secrets or full PII—redact.
Also applies to: 93-97, 159-163
🤖 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 `@src/openhuman/http_host/handlers.rs` around lines 65 - 70, The warning logs
currently print absolute filesystem paths via resolved.display(), which can leak
PII; update each log::warn! that logs the path (the Err branches where you log
"metadata failed path=... err=..." and similar occurrences later in the file) to
call redact_path_for_log(&resolved) instead of resolved.display(), and ensure
you pass the redacted string into the log macro (e.g., log::warn!("{} metadata
failed path={} err={}", LOG_PREFIX, redact_path_for_log(&resolved), error)).
Apply the same replacement for the other two occurrences mentioned (the similar
log::warn! blocks around the other metadata error handlers).
Comment on lines
+117
to
+128
| match tokio::fs::read_dir(dir).await { | ||
| Ok(mut entries) => { | ||
| let mut rows = Vec::new(); | ||
| while let Ok(Some(entry)) = entries.next_entry().await { | ||
| let name = entry.file_name().to_string_lossy().to_string(); | ||
| let file_type = match entry.file_type().await { | ||
| Ok(file_type) => file_type, | ||
| Err(_) => continue, | ||
| }; | ||
| let suffix = if file_type.is_dir() { "/" } else { "" }; | ||
| rows.push((name, suffix.to_string())); | ||
| } |
Contributor
There was a problem hiding this comment.
Don't silently return a partial directory listing on iteration errors.
while let Ok(Some(entry)) = entries.next_entry().await stops on the first read error and returns an incomplete listing as if it succeeded. This should log and fail the request instead.
Suggested fix
- while let Ok(Some(entry)) = entries.next_entry().await {
- let name = entry.file_name().to_string_lossy().to_string();
- let file_type = match entry.file_type().await {
- Ok(file_type) => file_type,
- Err(_) => continue,
- };
- let suffix = if file_type.is_dir() { "/" } else { "" };
- rows.push((name, suffix.to_string()));
- }
+ loop {
+ let entry = match entries.next_entry().await {
+ Ok(Some(entry)) => entry,
+ Ok(None) => break,
+ Err(error) => {
+ log::warn!(
+ "{LOG_PREFIX} read_dir iteration failed path={} err={}",
+ crate::openhuman::http_host::path_utils::redact_path_for_log(dir),
+ error
+ );
+ return (
+ StatusCode::INTERNAL_SERVER_ERROR,
+ "failed to enumerate hosted directory",
+ )
+ .into_response();
+ }
+ };
+ let name = entry.file_name().to_string_lossy().to_string();
+ let file_type = match entry.file_type().await {
+ Ok(file_type) => file_type,
+ Err(_) => continue,
+ };
+ let suffix = if file_type.is_dir() { "/" } else { "" };
+ rows.push((name, suffix.to_string()));
+ }📝 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
| match tokio::fs::read_dir(dir).await { | |
| Ok(mut entries) => { | |
| let mut rows = Vec::new(); | |
| while let Ok(Some(entry)) = entries.next_entry().await { | |
| let name = entry.file_name().to_string_lossy().to_string(); | |
| let file_type = match entry.file_type().await { | |
| Ok(file_type) => file_type, | |
| Err(_) => continue, | |
| }; | |
| let suffix = if file_type.is_dir() { "/" } else { "" }; | |
| rows.push((name, suffix.to_string())); | |
| } | |
| match tokio::fs::read_dir(dir).await { | |
| Ok(mut entries) => { | |
| let mut rows = Vec::new(); | |
| loop { | |
| let entry = match entries.next_entry().await { | |
| Ok(Some(entry)) => entry, | |
| Ok(None) => break, | |
| Err(error) => { | |
| log::warn!( | |
| "{LOG_PREFIX} read_dir iteration failed path={} err={}", | |
| crate::openhuman::http_host::path_utils::redact_path_for_log(dir), | |
| error | |
| ); | |
| return ( | |
| StatusCode::INTERNAL_SERVER_ERROR, | |
| "failed to enumerate hosted directory", | |
| ) | |
| .into_response(); | |
| } | |
| }; | |
| let name = entry.file_name().to_string_lossy().to_string(); | |
| let file_type = match entry.file_type().await { | |
| Ok(file_type) => file_type, | |
| Err(_) => continue, | |
| }; | |
| let suffix = if file_type.is_dir() { "/" } else { "" }; | |
| rows.push((name, suffix.to_string())); | |
| } |
🤖 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 `@src/openhuman/http_host/handlers.rs` around lines 117 - 128, The current
directory iteration using while let Ok(Some(entry)) = entries.next_entry().await
will silently stop on the first read error and return a partial listing; change
the loop to explicitly match on entries.next_entry().await and handle all arms:
Ok(Some(entry)) -> process and push to rows (keeping the existing file_type
handling), Ok(None) -> break the loop, Err(e) -> log the error and return an Err
(or propagate an appropriate HTTP error) so the request fails instead of
returning a partial listing; update the logic around entries, rows, and
entry.file_type().await to reflect this explicit match and error propagation.
This was referenced May 17, 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
http_hostdomain that can start, list, inspect, and stop ad-hoc static directory HTTP serversProblem
Solution
http_host.start,http_host.stop,http_host.get, andhttp_host.listhandlers under a dedicated Rust domainindex.htmlwhen present, otherwise render a simple directory listing, while rejecting traversal and canonicalized path escapeSubmission Checklist
diff-cover) meet the gate enforced by.github/workflows/coverage.yml. CI remains the source of truth for merged diff-cover; I did not run the full local coverage workflow for this core-only change.## Related— N/A: no matrix feature IDs were touched.docs/RELEASE-MANUAL-SMOKE.md) — N/A: core-only RPC surface, no release smoke doc change needed.Closes #NNNin the## Relatedsection — N/A: no GitHub issue was provided for this change.Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
codex/http-host-static-serverf05627bf3e0f02ddb92cd96fd7f4ae55f12eead2Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckcargo test --manifest-path Cargo.toml http_host:: --libcargo fmt --manifest-path Cargo.toml --all --check;cargo check --manifest-path Cargo.toml --libcargo fmt --manifest-path app/src-tauri/Cargo.toml --all --checkandcargo check --manifest-path app/src-tauri/Cargo.tomlValidation Blocked
command:node scripts/codex-pr-preflight.mjs --strict-path --lightweighterror:expected Codex-web path/workspace/openhuman, got/Users/enamakel/work/tinyhumansai/openhuman-6; branch naming convention expected an issue-key branch and this local change was shipped without a provided issue keyimpact:no code validation was blocked, but the PR metadata documents that this was a local checkout rather than a Codex-web issue branchBehavior Changes
Parity Contract
src/core/all.rs, params use schema-driven validation, and server shutdown is wired through the existing core shutdown hook mechanismDuplicate / Superseded PR Handling
Summary by CodeRabbit