feat: scope user data to per-user directories#370
feat: scope user data to per-user directories#370senamakel merged 8 commits intotinyhumansai:mainfrom
Conversation
…icated users - Implemented `read_authenticated_user_id` to extract the user's ID from `auth-profiles.json`, avoiding a dependency cycle with the credentials module. - Introduced `maybe_scope_workspace_to_user` to create user-specific workspace directories based on the authenticated user ID, ensuring isolated workspace data. - Updated the configuration loading process to call `maybe_scope_workspace_to_user`, enhancing user data management. - Added unit tests for the new functionality, ensuring correct behavior in various scenarios. This change improves user experience by providing personalized workspace management based on authentication status.
- Added functions to manage the active user state, including `read_active_user_id`, `write_active_user_id`, and `clear_active_user`, allowing for user-specific configuration and workspace isolation. - Introduced `default_root_openhuman_dir` to standardize the retrieval of the root directory for user data. - Updated configuration loading to support user-scoped directories, improving the overall user experience by ensuring personalized settings and workspace management. This change enhances the OpenHuman platform by enabling better user data management and isolation.
…torage - Added logic to create and activate user-scoped directories based on the resolved user ID when storing session data, ensuring credentials are saved in the correct location. - Implemented error handling for directory creation and active user ID writing, with appropriate logging for failures. - Updated the configuration loading process to reflect the newly activated user directory, improving user-specific settings management. - Enhanced the `get_data_dir` function to return user-scoped directories if an active user is set, streamlining data access. This change improves user experience by ensuring that session data is correctly organized and accessible based on user context.
- Renamed and refactored tests to better reflect functionality, focusing on active user ID management. - Removed the `write_auth_profiles` helper function and replaced it with direct calls to `write_active_user_id` for clarity. - Enhanced tests to cover scenarios for reading and clearing active user IDs, ensuring accurate behavior in user-specific configurations. - Added a new test for building user directory paths, improving overall test coverage for user management features. This change streamlines the testing process and enhances the clarity of user ID handling in the configuration schema.
- Introduced a new `shared_root_dir` function to centralize the logic for determining the shared root openhuman directory, improving code clarity and reducing duplication. - Updated `workspace_ollama_dir` and `workspace_local_models_dir` functions to utilize the new shared root directory, ensuring consistent path resolution for user-specific and shared resources. - Enhanced the `model_artifact_path` function to leverage the new directory structure, improving the organization of model artifacts. This refactor enhances maintainability and clarity in the path management for local AI resources.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 2 seconds. ⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces active user management to OpenHuman, enabling user-scoped configuration directories and automatic user switching. Configuration resolution now prioritizes user-specific settings from Changes
Sequence DiagramsequenceDiagram
participant Client
participant CredOps as Credential Ops
participant FileSystem as File System
participant Config as Config System
participant AuthService
Client->>CredOps: store_session(user_id)
CredOps->>CredOps: resolve user_id
CredOps->>FileSystem: create_dir_all(~/.openhuman/users/{user_id})
FileSystem-->>CredOps: directory created
CredOps->>FileSystem: write active_user.toml with user_id
FileSystem-->>CredOps: marker written
CredOps->>Config: load_config_with_timeout()
Config->>FileSystem: read active_user.toml
FileSystem-->>Config: user_id found
Config->>Config: resolve to ~/.openhuman/users/{user_id}
Config-->>CredOps: effective_config with user paths
CredOps->>AuthService: from_config(effective_config)
AuthService->>FileSystem: write credentials to user-scoped location
FileSystem-->>AuthService: written
AuthService-->>CredOps: session stored
CredOps-->>Client: success response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/openhuman/encryption/core.rs (1)
124-124: Outdated docstring: path is now dynamic.The docstring still references a fixed path
~/.openhuman/encryption.key, butget_key_file_path()now returns a user-scoped path when an active user is set (e.g.,~/.openhuman/users/{user_id}/encryption.key).📝 Suggested docstring update
-/// Get the path to the encryption key file (~/.openhuman/encryption.key). +/// Get the path to the encryption key file (user-scoped when an active user is set). fn get_key_file_path() -> Result<PathBuf, String> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/encryption/core.rs` at line 124, Update the outdated docstring for get_key_file_path() to reflect that the path is dynamic and user-scoped when an active user exists (e.g., returns ~/.openhuman/users/{user_id}/encryption.key) instead of the hard-coded ~/.openhuman/encryption.key; locate the get_key_file_path function and change its comment to describe both behaviors (global default path and per-user path when an active user is set) and include the example user-scoped path format.src/openhuman/config/schema/load.rs (1)
304-314: Consider atomic write foractive_user.toml.Unlike
persist_active_workspace_config_dir(lines 126-143) which uses a temp file + rename pattern for atomic writes,write_active_user_idwrites directly to the target file. If the process crashes mid-write, this could leave a corruptedactive_user.toml, causing subsequent reads to fail silently (returningNone).While the impact is limited (user would just need to re-login), consider using the same atomic write pattern for consistency:
♻️ Atomic write pattern
pub fn write_active_user_id(default_openhuman_dir: &Path, user_id: &str) -> Result<()> { let path = default_openhuman_dir.join(ACTIVE_USER_STATE_FILE); + let temp_path = default_openhuman_dir.join(format!( + ".{ACTIVE_USER_STATE_FILE}.tmp-{}", + uuid::Uuid::new_v4() + )); let state = ActiveUserState { user_id: user_id.to_string(), }; let toml_str = toml::to_string_pretty(&state).context("serialize active_user.toml")?; - std::fs::write(&path, toml_str) - .with_context(|| format!("Failed to write active user state: {}", path.display()))?; + std::fs::write(&temp_path, &toml_str) + .with_context(|| format!("Failed to write temp active user state: {}", temp_path.display()))?; + std::fs::rename(&temp_path, &path).with_context(|| { + let _ = std::fs::remove_file(&temp_path); + format!("Failed to atomically persist active user state: {}", path.display()) + })?; tracing::debug!(user_id = %user_id, path = %path.display(), "active user written"); Ok(()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/config/schema/load.rs` around lines 304 - 314, write_active_user_id currently writes active_user.toml directly which can corrupt the file if interrupted; change it to the atomic temp-file-then-rename pattern used by persist_active_workspace_config_dir: serialize the ActiveUserState to toml, write to a temporary file in the same directory (e.g., path.with_extension("tmp") or path.join(".tmp...")), fsync the temp file/parent if desired, then atomically rename the temp into place using std::fs::rename, and propagate contextual errors with .with_context() and preserve the same tracing::debug call after successful rename; update function write_active_user_id to mirror that pattern for safe atomic replacement.
🤖 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/openhuman/credentials/ops.rs`:
- Around line 117-124: The code currently sets effective_config to the original
config when resolved_user_id.is_some() but load_config_with_timeout() fails,
which can cause credentials to be written to the root config instead of the
user-scoped one; change the logic around effective_config (and the call sites
write_active_user_id() / load_config_with_timeout()) so that if
write_active_user_id() succeeded but load_config_with_timeout() returns Err you
do not silently fall back — instead propagate/return an error (or at minimum log
a clear warning) and abort the operation that writes credentials; specifically,
update the match on load_config_with_timeout() to return Err or call
processLogger/error-equivalent with context mentioning resolved_user_id and
refrain from using config.clone() as the effective_config when load fails.
---
Nitpick comments:
In `@src/openhuman/config/schema/load.rs`:
- Around line 304-314: write_active_user_id currently writes active_user.toml
directly which can corrupt the file if interrupted; change it to the atomic
temp-file-then-rename pattern used by persist_active_workspace_config_dir:
serialize the ActiveUserState to toml, write to a temporary file in the same
directory (e.g., path.with_extension("tmp") or path.join(".tmp...")), fsync the
temp file/parent if desired, then atomically rename the temp into place using
std::fs::rename, and propagate contextual errors with .with_context() and
preserve the same tracing::debug call after successful rename; update function
write_active_user_id to mirror that pattern for safe atomic replacement.
In `@src/openhuman/encryption/core.rs`:
- Line 124: Update the outdated docstring for get_key_file_path() to reflect
that the path is dynamic and user-scoped when an active user exists (e.g.,
returns ~/.openhuman/users/{user_id}/encryption.key) instead of the hard-coded
~/.openhuman/encryption.key; locate the get_key_file_path function and change
its comment to describe both behaviors (global default path and per-user path
when an active user is set) and include the example user-scoped path format.
🪄 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
Run ID: 47713770-b531-4178-9f15-3ad49c63dd07
📒 Files selected for processing (7)
src/openhuman/config/mod.rssrc/openhuman/config/schema/load.rssrc/openhuman/config/schema/mod.rssrc/openhuman/credentials/ops.rssrc/openhuman/encryption/core.rssrc/openhuman/local_ai/core.rssrc/openhuman/local_ai/paths.rs
| let effective_config = if resolved_user_id.is_some() { | ||
| match crate::openhuman::config::load_config_with_timeout().await { | ||
| Ok(c) => c, | ||
| Err(_) => config.clone(), | ||
| } | ||
| } else { | ||
| config.clone() | ||
| }; |
There was a problem hiding this comment.
Silent fallback may store credentials in the wrong directory.
If write_active_user_id succeeds but load_config_with_timeout fails, the code silently falls back to the original config. This could cause credentials to be stored in the root directory instead of the user-scoped directory, creating a security/isolation issue.
Consider returning an error or at least logging a warning when the reload fails after successfully activating the user directory:
🔧 Suggested improvement
let effective_config = if resolved_user_id.is_some() {
match crate::openhuman::config::load_config_with_timeout().await {
Ok(c) => c,
- Err(_) => config.clone(),
+ Err(e) => {
+ tracing::warn!(
+ error = %e,
+ "failed to reload config after user activation; credentials may be stored in wrong location"
+ );
+ config.clone()
+ }
}
} else {
config.clone()
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/credentials/ops.rs` around lines 117 - 124, The code currently
sets effective_config to the original config when resolved_user_id.is_some() but
load_config_with_timeout() fails, which can cause credentials to be written to
the root config instead of the user-scoped one; change the logic around
effective_config (and the call sites write_active_user_id() /
load_config_with_timeout()) so that if write_active_user_id() succeeded but
load_config_with_timeout() returns Err you do not silently fall back — instead
propagate/return an error (or at minimum log a clear warning) and abort the
operation that writes credentials; specifically, update the match on
load_config_with_timeout() to return Err or call processLogger/error-equivalent
with context mentioning resolved_user_id and refrain from using config.clone()
as the effective_config when load fails.
- Updated the `model_artifact_path` function to utilize a new `shared_root_dir` function, which centralizes the logic for determining the root openhuman directory. - Enhanced the `config_root_dir` function to improve clarity and maintainability. - Adjusted the `workspace_ollama_dir` and `workspace_local_models_dir` functions to leverage the new shared directory logic, ensuring consistent path resolution across the application. These changes improve the organization of directory management and enhance the overall clarity of the codebase.
Summary
~/.openhuman/users/{user_id}/~/.openhuman/root and are not duplicated per useractive_user.tomlmarker file at root level to track the currently active user; login writes it, logout removes itMotivation
Previously, all users shared the same
~/.openhuman/directory. If a user logged out and another logged in, they would see each other's workspace data, memory, cron jobs, encryption keys, and auth profiles. This change isolates all user-specific state.Directory layout
Changes
src/openhuman/config/schema/load.rsactive_user.tomlread/write/clear;resolve_runtime_config_dirs()redirects entire openhuman_dir per user; 6 unit testssrc/openhuman/config/schema/mod.rssrc/openhuman/config/mod.rssrc/openhuman/credentials/ops.rsstore_session()creates user dir + writes marker;clear_session()removes markersrc/openhuman/encryption/core.rsget_data_dir()respects active usersrc/openhuman/local_ai/paths.rsshared_root_dir()keeps models/binaries sharedsrc/openhuman/local_ai/core.rsmodel_artifact_path()uses shared rootTest plan
cargo checkpasses for both core and Tauri shell~/.openhuman/users/{id}/🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes