Skip to content

fix(config): seal the channel credentials the secret list missed - #8677

Merged
senamakel merged 2 commits into
tinyhumansai:mainfrom
CodeGhost21:fix/312-config-secrets
Nov 12, 2026
Merged

senamakel merged 2 commits into
tinyhumansai:mainfrom
CodeGhost21:fix/312-config-secrets

Conversation

@CodeGhost21

@CodeGhost21 CodeGhost21 commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Summary

  • With secrets.encrypt = true, five channel credentials were still written to config.toml in plaintext, and a sealed value could reach the channel as enc2: ciphertext:
    • channels_config.email.password
    • yuanbao.app_secret and yuanbao.token
    • linq.webhook_secret and relay.upgrade_secret, which are new; the audit found them
  • All five are now in the one secret registry (secret_slots) that encrypt, decrypt and the UI snapshot share.
  • The other two defects reported in [core-shrink] bug: config secrets left in plaintext and leaked through the UI snapshot senamakel/openhuman#312, the load path and snapshot redaction, are already fixed on main. The audit and its evidence are below.

Problem

senamakel#312 (filed 2026-10-18) reported three defects. Audit on main 954bcd3a50:

Defect On main
1. Fields missing from the encrypt/decrypt list One registry now exists, 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) and hosting.api_key (:206). Still missing: email.password, yuanbao.app_secret, yuanbao.token. The connect flow keeps them in the credentials store, but channels/credentials.rs lets a value in config.toml win, so a hand-written one stayed plaintext.
2. load_from_config_path_unenriched never decrypted Fixed. It is now load_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).
3. Snapshot leaked keys Fixed. snapshot_config_json clears every registry slot, then runs mask_unsealed_credentials and a name-based backstop. snapshot_carries_no_credential covers it.

Solution

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_ciphertext
    • every_sealed_credential_round_trips_through_save_and_reload (needs the staged TinySecurity module): fills every slot, saves, asserts no plaintext on disk, reloads through Config::load_from_config_path, and asserts every value. This also pins defect 2.

    loader_snapshot_secrets_tests.rs raises its slot count from 33 to 38. Failing-first: with secrets.rs reverted, 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 count 33 != 38 twice).

  • 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 ## Related

  • No 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 #NNN in the ## Related section

Impact

Related


AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

Commit & Branch

  • Branch: fix/312-config-secrets
  • Commit SHA: see the PR head

Validation Run

  • pnpm --filter openhuman-app format:check: N/A, no frontend changes
  • pnpm typecheck: N/A, no frontend changes
  • Focused tests: 14 config-secret tests pass, with and without the TinySecurity module staged. A wider config:: run (staged) gave 992 passed and 4 failed. The 4 are modules::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.
  • Rust fmt/check (if changed): cargo fmt --all -- --check; clippy shows no warnings in touched files; check-openhuman-rust-layout, check-saas-ambient and check-process-statics are clean for the touched files
  • Tauri fmt/check (if changed): N/A

Validation Blocked

  • command: node scripts/ci/check-storage-schema.mjs (locally)
  • error: the local Xcode xcodebuild is broken, so its schema-dump build cannot find clang
  • impact: none for this diff, which touches no storage collections; CI runs it.

Behavior Changes

  • Intended behavior change: five channel credentials are sealed at rest and decrypted on load like every other registry secret.
  • User-visible effect: none for connect-flow users; a hand-written credential in config.toml is no longer stored or passed on in the wrong form.

Parity Contract

  • Legacy behavior preserved: config file format, RPC names and the credentials-store path are unchanged.
  • Guard/fallback/dispatch parity checks: the same secret_slots registry drives encrypt, decrypt and the snapshot.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): none
  • Canonical PR: this one
  • Resolution (closed/superseded/updated): N/A

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Security
    • Email, Yuanbao, Linq, and Relay credentials are now included in configuration secret handling.
    • Sealed credentials are returned as usable values rather than ciphertext. If encryption is enabled but unavailable, saving a credential reports an error and leaves its plaintext value unchanged.
    • Configuration files no longer contain plaintext secret values after successful saving.

CodeGhost21 and others added 2 commits November 12, 2026 11:04
`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>
@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The secret-slot inventory now includes five channel credentials. Tests cover encryption failures, loading sealed credentials, and saving and reloading secret values.

Changes: Channel credential secret handling

Layer / File(s) Summary
Add channel credentials to secret slots
crates/openhuman-core/src/config/schema/load/secrets.rs, crates/openhuman-core/src/config/ops/loader_snapshot_secrets_tests.rs
The secret-slot inventory adds the Linq webhook secret, email password, Yuanbao app secret and token, and optional Relay upgrade secret. The snapshot fixture adds Yuanbao and Relay configuration and expects 38 sealed credential fields.
Test channel credential handling
crates/openhuman-core/src/config/schema/load/secrets_tests.rs
Tests verify that encryption without a cipher reports the affected field and preserves plaintext, that loading sealed credentials does not return ciphertext, and that saving and reloading all secret slots preserves values without writing sentinel plaintext.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: High

Possibly related PRs

Suggested labels: rust-core, bug

Suggested reviewers: m3ga-mind, tinysweeper, senamakel

Merge Risk: Minimal · up to c2dce

This change makes the email, Yuanbao, Linq and Relay credentials encrypted at rest and decrypted on load, as intended. No defect introduced by the diff was found. CI shows failing lanes whose cause is not attributed to this change, so owners should confirm they also fail on main before merging.

Pre-merge checks: Passed 3 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Linked Issues check Inconclusive Linked issue #312 has three coding requirements: (1) one secret registry for encrypt, decrypt and snapshot, covering the listed fields; (2) decryption on every config load path; (3) snapshot redaction… Confirm that config/ops/loader/snapshot.rs clears every secret_slots entry and that no secret value reaches get_config. Confirm that every load path that returns a Config (including load_from_config_path_with_env and the branches.rs pre-log…
Passed Passed checks: 3 passed

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "fix(config): seal the channel credentials the secret list missed" matches the main change. The PR adds five channel credentials (email password, Yuanbao app secret and token, Linq webhook s…
Out of Scope Changes check Passed The whole-PR diff against the merge base changes two kinds of files. The first is config/schema/load/secrets.rs (+16 lines). Each added slot is one of the five channel credentials named in #312 and th…
Full details: Title check

Explanation

The title "fix(config): seal the channel credentials the secret list missed" matches the main change. The PR adds five channel credentials (email password, Yuanbao app secret and token, Linq webhook secret, Relay upgrade secret) to the shared secret_slots registry, and the raw summary shows matching changes in secrets.rs and tests. The title is short, specific, and follows a conventional commit format. It does not cover every detail, which is acceptable.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base changes two kinds of files. The first is config/schema/load/secrets.rs (+16 lines). Each added slot is one of the five channel credentials named in #312 and the PR, and each uses the existing SecretSlot pattern. The second is test code: secrets_tests.rs (+178 lines) and loader_snapshot_secrets_tests.rs (slot count 33 to 38, with Yuanbao and Relay fixture values). These support the registry change directly. No unrelated production code changes were found. The PR does not add the follow-up items (whatsapp.pair_code, MCP credentials) that its description lists as out of scope.

Full details: Linked Issues check

Explanation

Linked issue #312 has three coding requirements: (1) one secret registry for encrypt, decrypt and snapshot, covering the listed fields; (2) decryption on every config load path; (3) snapshot redaction, plus regression tests. Verified at head: the diff in config/schema/load/secrets.rs adds five slots to secret_slots: email.password, yuanbao.app_secret, yuanbao.token, linq.webhook_secret and relay.upgrade_secret. The registry already covers api_key, search..api_key, local_ai.api_key, composio.api_key, integrations..api_key and hosting.api_key, so requirement (1) for the encrypt and decrypt paths is supported. Verified in part: impl_load.rs at head passes a decrypt_secrets flag through load_or_init_with_env_lookup_mode, and load_from_default_paths calls decrypt_config_secrets. The load_from_config_path_with_env body was truncated, so the all-or-nothing strict decrypt path is not confirmed. Not verified: the snapshot path (config/ops/loader/snapshot.rs and snapshot_config_json) and the new tests in secrets_tests.rs and loader_snapshot_secrets_tests.rs. The PR description states that defects 2 and 3 were already on main, but the reviewed evidence does not confirm this for snapshot redaction.

Resolution

Confirm that config/ops/loader/snapshot.rs clears every secret_slots entry and that no secret value reaches get_config. Confirm that every load path that returns a Config (including load_from_config_path_with_env and the branches.rs pre-login path) calls the decrypt step. Confirm that the regression tests in secrets_tests.rs and loader_snapshot_secrets_tests.rs exist at head and assert the round-trip and snapshot behavior described in the PR.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the secret list,
And finds five fields that had been missed.
Sealed values pass, then turn back clear,
No sentinel leaves the file from here.
The bunny hops and guards the config tight.

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7590dd5a-171e-4b7a-abf6-a0a3af2d7eb9
Commits

Reviewing files that changed from the base of the PR and between 202a2e1 and c2dcee3.

Files selected for processing: 3

  • crates/openhuman-core/src/config/ops/loader_snapshot_secrets_tests.rs
  • crates/openhuman-core/src/config/schema/load/secrets.rs
  • crates/openhuman-core/src/config/schema/load/secrets_tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@CodeGhost21
CodeGhost21 marked this pull request as ready for review November 12, 2026 06:58
@tinysweeper

tinysweeper Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: c2dcee31b902c95da8df37e835c403526cc81c9e

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 3 of 3 files.
Pull request purpose (title): fix(config): seal th…

Diagnostics and supporting details

Full summary

Pull 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

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 2 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The 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

  • Modified — Email password sealed through the secret registry: With encryption enabled, a hand-written `channels_config.email.password` in `config.toml` is sealed on save and decrypted on load, and a save without an available cipher fails naming the field instead of writing plaintext. (crates/openhuman-core/src/config/schema/load/secrets.rs pub(crate) fn secret_slots(config: &mut Config) -> Vec<(String, SecretSlot<'_>)>)
  • Added — Yuanbao app_secret and token sealed through the secret registry: Both yuanbao credentials are now registered, so a value written by hand to `config.toml` is sealed on save, opened on load, and fails closed by field name when no cipher is available. (crates/openhuman-core/src/config/schema/load/secrets.rs pub(crate) fn secret_slots(config: &mut Config) -> Vec<(String, SecretSlot<'_>)>)
  • Added — Linq webhook_secret sealed through the secret registry: `linq.webhook_secret` is added alongside the existing `linq.api_token` slot, giving it the same encrypt/decrypt/snapshot treatment. (crates/openhuman-core/src/config/schema/load/secrets.rs pub(crate) fn secret_slots(config: &mut Config) -> Vec<(String, SecretSlot<'_>)>)
  • Added — Relay upgrade_secret sealed through the secret registry: The optional `relay.upgrade_secret` is registered as an optional slot, so it is sealed when present and decrypted on load, while remaining absent when unset. (crates/openhuman-core/src/config/schema/load/secrets.rs pub(crate) fn secret_slots(config: &mut Config) -> Vec<(String, SecretSlot<'_>)>)
  • Modified — Snapshot fixture covers the new channel slots: The fixture now sets yuanbao and relay channels so those slots are registered and filled, and the expected sealed-credential count rises from 33 to 38, keeping the snapshot leak scan aligned with the expanded registry. (crates/openhuman-core/src/config/ops/loader_snapshot_secrets_tests.rs fn config_with_every_secret() -> Config {)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 3 files; 0 findings. _The code index is behind this pull request (indexed at `b7021872ed5f`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 3 files; 0 findings. _The code index is behind this pull request (indexed at `b7021872ed5f`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change extends the single sealed-credential slot list with five channel fields (email password, yuanbao app_secret and token, linq webhook_secret, relay upgrade_secret) and adds tests that pin the save-side fail-closed error naming each field, the load-side clearing of undecryptable ciphertext, a full save/reload round trip, and an updated snapshot fixture whose slot count and leak scan now include the new fields. The coverage matches the behaviour and looks sound to merge. _The code index is behind this pull request (indexed at `b7021872ed5f`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The commits lane found no sensitive content in the committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `b7021872ed5f`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The description lane found no defect: it lists exactly the five added slots and notes the 33→38 count bump, and the yuanbao and relay fixture additions are the necessary precondition for those slots to be registered, while the save-without-cipher failure mode is documented under Impact.
  • Lane summary: The description matches the diff: it lists exactly the five slots added to `secret_slots` (`email.password`, `yuanbao.app_secret`, `yuanbao.token`, `linq.webhook_secret`, `relay.upgrade_secret`), describes the tests that were added or extended in `secrets_tests.rs`, and notes the 33→38 count bump in `loader_snapshot_secrets_tests.rs`. The fixture additions for the yuanbao and relay channels in that file are the necessary precondition for those two new slots to be registered, so the count claim stays accurate, and the save-without-cipher failure mode is called out in `## Impact` rather than hidden. No description defect found, and the code change itself is a consistent extension of the existing registry. _The code index is behind this pull request (indexed at `b7021872ed5f`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash, deepseek-v4-flash
  • Spend: $0.017141
  • Completion usage: incomplete; some provider completion costs are unavailable.
  • Tokens: 560958 input · 82038 output · 161152 cached · 404 embedding
  • Updated: 2026-11-12T07:02:24Z
Head State Pass summary
c2dcee31b902 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-11-12T07:02:24Z)

tinysweeper 0.1.2

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-12T07:03:29.948684Z c2dcee3 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai coderabbitai Bot added bug rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. labels Nov 12, 2026

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0171

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Nov 12, 2026
@senamakel
senamakel merged commit f0b874b into tinyhumansai:main Nov 12, 2026
24 of 44 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Team Openhuman Nov 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[core-shrink] bug: config secrets left in plaintext and leaked through the UI snapshot

2 participants