Skip to content

Pre-prod: config.toml written with insecure permissions (mode 644, world-readable) — auto-fixed at runtime but not at write time #5635

Closed

Description

@Al629176

Summary

On startup, OpenHuman detects that config.toml was created with mode 644 (world-readable) and auto-corrects it to 600. The file should be written with 600 permissions from the start — the auto-fix is a safety net, not a solution, because the file is briefly world-readable between creation and correction.

Log evidence (pre-prod, 2026-08-20)

16:03:05 WRN [config] Config file
"/Users/alan/.openhuman/users/69de186d2c2521812c1936d1/config.toml"
is world-readable (mode 644); auto-fixing to 600

Security impact

config.toml contains user configuration including API keys, model routing preferences, and potentially connection identifiers. Being world-readable (even briefly) on a multi-user system exposes this to any local process running as a different user. The auto-fix closes the window, but the write should use 0o600 from creation.

Fix direction

Use OpenOptions::new().mode(0o600) (or equivalent) when initially writing config.toml. Remove the startup auto-fix once the write permission is correct at source.

Activity

  1. Felyx-Fu commented on Aug 23, 2026

    @Felyx-Fu
    Contributor

    Static security triage (2026-08-23)

    Verdict: needs_review; likely remediated on current main but not release-closed.
    Evidence/gap: Current main writes the temporary config with 0600 permissions before rename and attempts to repair permissions on load. Verify the fix in the target release artifact and add platform-appropriate permission tests; do not claim every filesystem is covered while unsupported permission operations remain warning-only.

  2. M3gA-Mind commented on Aug 24, 2026

    @M3gA-Mind
    Collaborator

    How to fix this

    The write-time hardening this issue asks for already exists and has since 2026-07-25 (9f22ae408), so implementing the OpenOptions::mode(0o600) suggestion would be a no-op. The 644 state is nonetheless real and recurring, because a second writer — not Config::save() — rewrites config.toml afterwards. That writer is identified below; this issue should be re-scoped to it.

    Root cause: the memory-sources registry persists memory_sources by rewriting the entire config.toml through its own atomic-write path, which creates the temp file with File::create (mode 0o666 & ~umask = 0644) and renames it over the config. The rename carries 0644 onto the live file. It runs on every memory_sources_list RPC — not just at boot — so the load-time heal is undone within minutes of every launch, which is why the warning recurs indefinitely.

    Where:

    • vendor/tinymemory/crates/tinymemory-sources/src/registry.rs:166 — std::fs::File::create(&tmp_path), no mode set, no set_permissions anywhere in the file.
    • vendor/tinymemory/crates/tinymemory-sources/src/registry.rs:174 — std::fs::rename(&tmp_path, &self.path) carries that mode onto the target.
    • self.path is the user's config.toml: tinymemory-core/src/sources/registry.rs:30 constructs SourceRegistry::new(config.config_path().clone()).
    • Call chain: src/openhuman/memory/sources/rpc.rs:156 → tinymemory-core/src/sources/reconcile.rs:63 → tinymemory-sources/src/registry.rs:133 (write_all) → :148 (atomic_write).
    • Same unhardened copy exists at vendor/tinycortex/src/memory/sources/registry.rs:163.

    This is a vendored submodule, so the change lands in the tinymemory repo plus a submodule bump here — not in src/.

    The change: in atomic_write, harden the temp file before writing bytes, mirroring what Config::save() already does at src/openhuman/config/schema/load/impl_load.rs:671-681. Under #[cfg(unix)], after File::create and before write_all, set_permissions(&tmp_path, Permissions::from_mode(0o600)). Keep it non-fatal on failure (warn and continue) exactly as the core does — filesystems without chmod (CIFS/SMB, exFAT, some FUSE) must not turn a working save into a hard error. atomic_write is the single choke point for this file, so one edit covers write_all and every mutation RPC that reaches it.

    How to prove it: a unit test in tinymemory-sources that points a SourceRegistry at a temp config.toml pre-created at 0o644, calls upsert_composio_sources_batch, then asserts metadata.permissions().mode() & 0o077 == 0. It fails today (the rename re-widens to 0644) and passes with the hardening. Gate it #[cfg(unix)]. Worth a second case asserting the pre-existing file's mode is not inherited, since the failure is in the temp file rather than the target.

    Forensic confirmation that this is the writer, from a machine running a build of this tree:

    config.toml : mode=644 birth=08-21 21:12:05 mtime=21:12:05 inode=137766671
    config.toml.bak : mode=600 mtime=08-21 21:10:26 inode=108719366
    21:10:26 INF [rpc] openhuman.config_update_meet_settings -> ok   <- Config::save(), wrote .bak, left 600
    21:12:05 INF [memory_sources:reconcile] composio reconciliation complete targets=2 upserted=2
    

    Config::save() always copies to .bak before renaming; atomic_write never does. The config was re-created as a new inode at 644 with .bak untouched — so the 21:12:05 writer was atomic_write, not save().

    Risk / blast radius: atomic_write is private to SourceRegistry and only ever writes config.toml, so the change cannot affect other files. The real risk is the chmod itself failing on a permission-less filesystem — hence non-fatal. Note the tinycortex copy at the line above is a separate crate; check whether it is reachable in any build before deciding to fix or delete it.

    Not covered by this:

    • The issue's suggestion to remove the startup auto-fix should not be taken. It is what repairs configs already sitting at 644 on disk from earlier versions; deleting it would strand them permanently. Keep it as the heal, and let it go quiet once the writer stops re-widening.
    • Actual secret exposure via config.toml is lower than the issue assumes: on the machine inspected it contained zero enc2: values. Provider keys live in auth-profiles.json.
    • That file is the more serious problem and is not covered here. auth-profiles.json is written by src/openhuman/security/credentials/profiles.rs:1044 (fs::write(&tmp_path, ...), 0644) and renamed at :1060, with zero set_permissions calls anywhere in src/openhuman/security/credentials/ and no load-time heal to compensate. It holds the provider credential store and is 0644 on disk right now. Same bug class, worse contents, no safety net — it warrants its own issue at higher severity rather than being folded in here.

    Confidence: high — the writer is identified by code path and corroborated by inode/mtime forensics that discriminate it from Config::save(). Medium only on whether the tinycortex duplicate is live; that needs a dependency check, not a guess.

    — fleet research, verified against main @ e1c332b

  3. M3gA-Mind commented on Aug 24, 2026

    @M3gA-Mind
    Collaborator

    Fix raised: tinyhumansai/tinymemory#91 — fix(sources): create the registry temp file owner-only so a mutation cannot re-widen it. CI: 22 passing, MERGEABLE.

    The fix is not in this repo. The write-time 0600 this issue asks for already landed in 9f22ae408 (#5190); the file was being re-widened afterwards by a registry temp file created with default permissions in tinymemory. That is what PR 91 fixes.

    Because the PR lives in another repository it carries no closing keyword — a bare Closes #5635 there would resolve against tinymemory's own issue #5635, not this one. This issue must be closed by hand once PR 91 merges and the tinymemory submodule is bumped here.

    — fleet, verified against main @ e1c332b

  4. moved this from Todo to Need review in Team Openhumanon Aug 24, 2026
  5. M3gA-Mind commented on Aug 25, 2026

    @M3gA-Mind
    Collaborator

    Update: the fix is merged upstream, but not yet in openhuman.

    tinymemory#91 is merged into tinymemory main — the registry temp file that was re-widening config.toml to 644. It reaches this repo only through a vendor/tinymemory submodule bump, which has not happened yet.

    The bump is not a pointer move. openhuman currently pins v1.3.0 (d9a8a6eaf); tinymemory main is 26 commits ahead across three releases. v1.4.0 added two required trait methods with no default bodies (recall_namespace_recent, insert_event), so five host impls stop compiling without new code. Verified by compiler probe, not inference:

    error[E0407]: method `recall_namespace_recent` is not a member of trait `MemoryRetrieval`
    

    So the gitlink bump and the host impls must land in one atomic PR — either half alone leaves main red. That work is planned (TRAIT-IMPL-PLAN.md) and pending a version decision between v1.4.0 and v1.5.0.

    Close this issue by hand once that bump lands. The PR carries no closing keyword, correctly: a bare Closes #5635 in tinymemory would have resolved against that repo's own numbering.

    — fleet

  6. M3gA-Mind commented on Aug 26, 2026

    @M3gA-Mind
    Collaborator

    Fixed and now delivered to openhuman.

    tinymemory#91 fixed the registry temp file that was re-widening config.toml to 644, and the fix reached this repo when #5725 merged (77fddf591) and moved the vendor/tinymemory gitlink from d9a8a6eaf (v1.3.0) to 413b07c03 (v1.12.0).

    Verified by ancestry rather than by version number, because a release tag is not a reliable proxy here — neither v1.4.0 nor v1.5.0 contained these fixes, since both merged after v1.5.0 was cut:

    compare/<91 merge sha>...413b07c03  ->  status=ahead, behind_by=0
    

    behind_by=0 is the part that matters: the pin contains the fix.

    Closing by hand because the fixing PR lives in another repository and could not carry a closing keyword — a bare Closes #5635 there would have resolved against tinymemory's own issue numbering.

  7. moved this from Need review to Done in Team Openhumanon Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugpre prod buildBugs found during pre-production build testing

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions