Repository navigation
fix(config): seal the channel credentials the secret list missed - #8677
Conversation
`secret_slots` drives save, load and the UI snapshot, but it skipped `email.password`, `yuanbao.app_secret`, `yuanbao.token`, `linq.webhook_secret` and `relay.upgrade_secret`. The connect flow keeps the email and Yuanbao secrets in the credentials store, but a value in `config.toml` still wins, so with `secrets.encrypt = true` these stayed in plaintext on disk and a sealed value reached the channel as `enc2:` ciphertext. They are now in the list. Tests pin that each one is sealed on save and opened on load, and that every listed credential round-trips through save and the snapshot reload (`load_from_config_path`). Refs tinyhumansai#312 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
Tiny Sweeper reviewReviewed head: Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): fix(config): seal the channel credentials the secret list missed. This pull request adds five channel credential fields to the single secret registry (`secret_slots`) so that with `secrets.encrypt = true` they are sealed on save and decrypted on load instead of being written to `config.toml` as plaintext or handed to channels as `enc2:` ciphertext: `email.password`, `yuanbao.app_secret`, `yuanbao.token`, `linq.webhook_secret`, and `relay.upgrade_secret`. It also extends the snapshot fixture to include the yuanbao and relay channels and raises the expected sealed-slot count from 33 to 38, and adds tests pinning fail-closed save behaviour, load-side clearing of undecryptable ciphertext, and a save/reload round trip. The latest-push-delta evidence is unavailable. Review snapshot
Completeness: Complete What changedThe registry function now includes the email password, both yuanbao secrets and token, the linq webhook secret, and the optional relay upgrade secret, with a comment noting that hand-written values in `config.toml` override the credentials store and therefore must be sealed. The snapshot fixture populates the yuanbao and relay channels so the two new slots exist, and its filled-slot assertion moves from 33 to 38. New tests exercise each channel credential individually and as a set. Features
TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Before mergeNone. Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Summary
secrets.encrypt = true, five channel credentials were still written toconfig.tomlin plaintext, and a sealed value could reach the channel asenc2:ciphertext:channels_config.email.passwordyuanbao.app_secretandyuanbao.tokenlinq.webhook_secretandrelay.upgrade_secret, which are new; the audit found themsecret_slots) that encrypt, decrypt and the UI snapshot share.Problem
senamakel#312 (filed 2026-10-18) reported three defects. Audit on main
954bcd3a50:secret_slots(config/schema/load/secrets.rs:170), used by encrypt (:141), decrypt (:102, strict :60) and the snapshot. These are covered:api_key(:173),search.tinyfish.api_key(:184),local_ai.api_key(:191),composio.api_key(:192),integrations.*.api_key(:193-204) andhosting.api_key(:206). Still missing:email.password,yuanbao.app_secret,yuanbao.token. The connect flow keeps them in the credentials store, butchannels/credentials.rslets a value inconfig.tomlwin, so a hand-written one stayed plaintext.load_from_config_path_unenrichednever decryptedload_from_config_path_with_env(impl_load.rs:359), which decrypts all-or-nothing (decrypt_config_secrets_strict, :409-421). All its callers and the other load paths decrypt too (branches.rs:249,impl_load.rs:325, the preflight path at :147).snapshot_config_jsonclears every registry slot, then runsmask_unsealed_credentialsand a name-based backstop.snapshot_carries_no_credentialcovers it.Solution
secrets.rs(+16 lines): five slots added. Diff kept small because feat(inference): add OCI Generative AI as a cloud provider #8183, fix(providers): send custom headers, a product User-Agent and the OpenCode Go session to cloud providers #8059 and feat(search): add Keenable to search settings (works without a key) #7279 also add fields here.whatsapp.pair_codemcp_client.registry_auth.{smithery_api_key, mcp_official_token}(tinymcp)auth, env and argsEphemeralRoute.api_keyandSecurityJudgeRoute.api_keyare never serialized.Submission Checklist
Tests added or updated (happy path + at least one failure / edge case). In
secrets_tests.rs:a_channel_credential_is_never_saved_in_plaintext: with no cipher, saving fails and names the field.a_sealed_channel_credential_never_reaches_callers_as_ciphertextevery_sealed_credential_round_trips_through_save_and_reload(needs the staged TinySecurity module): fills every slot, saves, asserts no plaintext on disk, reloads throughConfig::load_from_config_path, and asserts every value. This also pins defect 2.loader_snapshot_secrets_tests.rsraises its slot count from 33 to 38. Failing-first: withsecrets.rsreverted, 5 tests fail (email.password must be sealed on save, not written in plaintext,email.password reached the caller sealed: enc2:…,email.password unlisted, and the slot count33 != 38twice).Diff coverage ≥ 80%: N/A: not measured locally. The change is the 16 registry lines, all exercised by the round-trip and snapshot tests; the CI gate measures it.
Coverage matrix updated: N/A: behaviour-only change to existing config-secret rows.
All affected feature IDs from the matrix are listed under
## RelatedNo new external network dependencies introduced
Manual smoke checklist updated if this touches release-cut surfaces: N/A: no UI or release-cut surface changes
Linked issue closed via
Closes #NNNin the## RelatedsectionImpact
loader_snapshot_secrets_tests.rs(now 38).Related
whatsapp.pair_codeand the MCP registry and server credentials.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/312-config-secretsValidation Run
pnpm --filter openhuman-app format:check: N/A, no frontend changespnpm typecheck: N/A, no frontend changesconfig::run (staged) gave 992 passed and 4 failed. The 4 aremodules::computer_config::tests::*("could not admit the secrets directory: Not a directory"), which fail the same way unstaged and with the new test skipped, so it is the local environment.cargo fmt --all -- --check; clippy shows no warnings in touched files;check-openhuman-rust-layout,check-saas-ambientandcheck-process-staticsare clean for the touched filesValidation Blocked
command:node scripts/ci/check-storage-schema.mjs(locally)error:the local Xcodexcodebuildis broken, so its schema-dump build cannot find clangimpact:none for this diff, which touches no storage collections; CI runs it.Behavior Changes
config.tomlis no longer stored or passed on in the wrong form.Parity Contract
secret_slotsregistry drives encrypt, decrypt and the snapshot.Duplicate / Superseded PR Handling
🤖 Generated with Claude Code
Summary by CodeRabbit