fix: require user approval for doctor write/delete operations#79
fix: require user approval for doctor write/delete operations#79
Conversation
OpenClaw updated its secrets management to use SecretRef objects
({ source, provider, id }) in auth-profiles.json instead of plaintext
strings. This adds compatibility so profile sync from VPS/local can
resolve credentials through the new scheme.
- Add SecretRef parsing and resolution for env/file sources
- Parameterize credential extraction with env resolver for local vs remote
- Update RemoteAuthCache to discover and batch-fetch SecretRef env vars
- Update per-profile fallback to resolve SecretRef env vars via SSH
- Fix pre-existing test import in agent_fallback.rs
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract check_entry closure to free function collect_secret_ref_env_names_from_entry - Add eprintln diagnostic for non-absolute SecretRef file paths - Run cargo fmt --all to fix formatting (CI failure) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
isDoctorAutoSafeInvoke was auto-approving mutative commands (config writes, file writes, session deletes) without user consent. Narrow the safe list to read-only diagnostic commands only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dev01lay2
left a comment
dev01lay2
left a comment
There was a problem hiding this comment.
No CI checks reported yet — please wait for CI before merging.
The core change in use-doctor-agent.ts is correct and important: auto-approving write/delete/upsert doctor operations was unsafe, and restricting auto-approve to read-only diagnostics is the right call.
Blocking
get_ssh_transfer_stats was dropped from the Tauri invoke handler in src-tauri/src/lib.rs. The diff shows it removed from the use crate::commands::{...} import block that feeds invoke_handler!, but it is not added anywhere else. Any frontend call to get_ssh_transfer_stats will fail at runtime with a command-not-found error. If this was intentional (renamed or moved), please explain; otherwise restore it.
Non-blocking
NBS: resolve_secret_ref_with_env silently ignores source values other than env and file. A tracing::warn! for unrecognised sources would help users debug misconfigured secret refs instead of getting silent failures.
dev01lay2
left a comment
dev01lay2
left a comment
There was a problem hiding this comment.
APPROVED ✅
The headline change — narrowing isDoctorAutoSafeInvoke to read-only commands only — is correct and important. Auto-approving mutative ops (config-upsert, file write, sessions-delete, config set/delete/unset) was a real foot-gun; users now get a chance to intercept any destructive doctor suggestion. The remaining auto-safe list looks genuinely read-only.
The SecretRef credential resolution landing is also solid. RemoteAuthCache.find_in_auth_stores now correctly routes through env_lookup instead of the local-env variant, and the build reorder (auth stores → scan SecretRef names → single batch env read) is efficient and correct.
NBS x2 (carried over):
eprintln!inresolve_secret_ref_fileshould go throughtracing::warn!/log::debug!— bypasses the structured log pipeline["token", "key", "apiKey", "api_key", "access"]is duplicated betweencollect_secret_ref_env_names_from_entryandextract_credential_from_auth_entry_with_env— a new credential field added to extraction won't automatically be collected for remote env batch reads; consider a shared constant
No CI checks reported yet — worth waiting for green before merge.
2 participants
Summary
isDoctorAutoSafeInvoketo only auto-approve read-only diagnostic commandsconfig-upsert,config-delete,file write,sessions-upsert,sessions-delete,fix-openclaw-path,config set,config delete,config unset) from the auto-safe listTest plan
npm run buildpasses (verified)probe-openclaw,file read,config-read,config get, etc.) still auto-approve as before🤖 Generated with Claude Code