fix(profiles): prevent credential leaks and SQLite data loss in exports

[keiranhaax]

Summary

Current main includes #83458, which force-redacts recognized secret-shaped strings in supported text files before profile archives are written. That substantially reduces risk, but it does not fully enforce the profile-export security boundary.

Named-profile exports still exclude only the exact files auth.json and .env. Backups, renamed credential stores, opaque authorization state, private-key formats, binary databases, and sensitive caller-supplied extras can still enter a portable archive without matching the text redactor.

This PR complements #83458 by adding structural path/type filtering before archive creation while retaining its final text-scrubbing pass.

What this fixes

• Excludes renamed and backup credential files at every copy depth.
• Excludes canonical OAuth, token, pairing, and profile-home credential trees.
• Excludes private PEM, PuTTY .ppk, and keystore formats while preserving public certificates.
• Blocks sensitive paths supplied through extra_files.
• Covers Feishu pairing state, reason-labelled credential-directory backups, and Hermes-generated corrupt-config backups.
• Scrubs every explicitly portable dotenv template.
• Creates consistent snapshots of live .db, .sqlite, and .sqlite3 databases before omitting WAL/SHM/journal sidecars.
• Preserves safe negative controls such as public certificates, ordinary same-named directories, desktop metadata, and non-sensitive rules files.

Security impact

Profile exports are intended to be portable and shareable. Exploitation requires a user to create and expose an archive, so this is not a passive remote vulnerability. However, affected archives can contain live credentials, private keys, pairing authorization state, or inconsistent database contents. Avoiding or manually sanitizing exports is the current workaround.

Validation

• 125 focused profile-export tests passed.
• 100 profile/distribution tests passed, 2 skipped.
• 103 SharedMetricsStore/SQLite-backup tests passed, 7 skipped.
• Final immutable review run: 373 passed, 2 skipped.
• Ruff, compilation, diff checks, and adversarial archive probes passed.
• Final independent exact-SHA review found no blockers.

The branch is 3 commits ahead, 0 behind current main, with contributor authorship preserved and no merge conflicts.

[liuhao1024]

Verified the scope and correctness of this fix. A few specific confirmations:

Both export paths covered: The _is_sensitive_export_name() classifier is used by _default_export_ignore() (root-level ~/.hermes export) AND the named-profile shutil.copytree(ignore=_named_ignore) path. No code path can drift.

Regex avoids false positives on non-credential files: _EXPORT_CREDENTIAL_KEYWORD_RE uses word-boundary matching ((?:^|[._-]) ... (?:$|[._-])), so tokenizer.json, token_count.md, and secret-santa.md are correctly excluded. The suffix gate (.json, .txt, .yaml, etc.) further narrows matches — my-secrets-notes.md is safe because .md isn't in _EXPORT_CREDENTIAL_CONTAINER_SUFFIXES.

Case-insensitive: the lowered = name.lower() entry point handles Config.YAML.BAK.* and AUTH.JSON variants.

Nested backups caught: the ignore callback runs at every shutil.copytree depth, so skins/old/config.yaml.bak.20260101 is excluded too — confirmed by test_named_profile_export_excludes_backups_and_secrets.

LGTM.

[tonydwb]

Code Review Summary

Verdict: Approved ✅

Review

This PR fixes a genuine security gap where profile exports could leak credential backups (config.yaml.bak*, .env.bak*, etc.) that were not caught by the old exclusion list.

✅ Looks Good

• Comprehensive approach: Single _is_sensitive_export_name() helper used by both default and named profile export paths, preventing future drift.
• Well-considered edge cases: Distinguishes .env.example (safe) from .env.local (sensitive), handles case-insensitivity, credential keyword matching bounded by delimiters to avoid false positives on tokenizer.json.
• Thorough test coverage: Parametrized unit tests for 25+ sensitive and safe name patterns, plus integration tests for both export paths verifying nested backup exclusion.
• Clean diff: 124 additions, only 4 deletions — minimal change to existing logic, mostly new helper + tests.
• No security concerns detected: No hardcoded secrets, no SQL injection vectors, no path traversal issues.

Checklist Summary

Category	Status
Correctness	✅ Edge cases handled (nested backups, case-insensitive, keyword boundary)
Security	✅ Fixes the exact security gap described
Code Quality	✅ Clean helper, single source of truth, well-documented
Testing	✅ Unit + integration, sensitive and safe paths, both export modes
Performance	✅ O(n) per export, no concerns

---

Reviewed by Hermes Agent (cron job)

[tonydwb]

Code Review Summary

Verdict: Approved ✅ — Fix profile export to exclude backup credentials from exports. Well-scoped, no security concerns.

---

Reviewed by Hermes Agent

[keiranhaax]

Refreshed this PR against current main.

Current state:

• Branch is now 1 commit ahead, 0 behind main.
• Reapplied the profile export credential-backup hardening cleanly on the current hermes_cli/profiles.py export path.
• Verified both default and named profile export paths still use the shared sensitive-file classifier.

Focused tests passed locally:

uv run --with pytest python -m pytest tests/hermes_cli/test_profile_export_credentials.py tests/hermes_cli/test_profiles.py -o 'addopts=' -q
# 208 passed in 1.62s

Ready for maintainer review.

[teknium1]

Thanks for addressing a real export-security gap: current named-profile export only excludes exact .env and auth.json entries (hermes_cli/profiles.py:1932-1942), so backup files can enter the archive.

Problems

• The proposed classifier remains incomplete for credential material. Hermes' canonical managed-files guard treats .anthropic_oauth.json, google_token.json, google_oauth_pending.json, google_oauth.json, webhook_subscriptions.json, and bws_cache.json as sensitive (hermes_cli/web_server.py:1303-1316), and treats mcp-tokens/ and pairing/ as credential-directory trees (hermes_cli/web_server.py:1318-1330). Those names/trees are not covered by the added classifier, so named-profile exports can still include them.
• The added archive tests do not exercise those canonical OAuth files or credential directories.

Suggested changes

• Apply the canonical sensitive basename and directory policy at every export-copy depth for both export modes, then add archive assertions for the OAuth stores and mcp-tokens/ / pairing/ trees.
• Reconcile the salvage with current main's root allow-list in hermes_cli/profiles.py:1863-1897; GitHub currently reports this PR as conflicting.

Automated hermes-sweeper review.

[keiranhaax]

Updated this PR against current main and addressed the Hermes Sweeper feedback.

Changes:

• Added the canonical OAuth and credential stores, plus mcp-tokens/ and pairing/, to export filtering at every copy depth.
• Excluded backup, timestamped, and renamed copies of sensitive files and credential trees.
• Reconciled the default-profile export with the current root allow-list while preserving the exact safe dotenv templates.
• Refined PEM handling to stream-scan direct and backup PEM files for private-key headers, while preserving public certificates and CA bundles.
• Added archive-level regression coverage for both default and named-profile exports.

Validation:

uv run --with pytest python -m pytest tests/hermes_cli/test_profile_export_credentials.py tests/hermes_cli/test_profiles.py -o 'addopts=' -q
# 231 passed

The branch is now 1 commit ahead and 0 behind main, with conflicts resolved. Ready for maintainer review.

[DiscoStew6082]

External review — blockers found on exact head 75c63841bc5aa01f4c062aeea672824a039e0967

The structural filtering and WAL-consistent SQLite snapshot work are valuable, and the staged extra_files symlink overwrite is fixed. However, five reproducible blockers remain:

1. _snapshot_export_sqlite_databases() preserves opaque credentials contained in state.db and other SQLite stores. A shareable export containing such a database still leaks those values.
2. _scrub_export_secrets() only processes an extension/name allowlist. Secret-bearing UTF-8 in desktop.data, .log, and extensionless files remains plaintext, including through REST extra_files.
3. _make_profile_archive() follows an existing symlink at the caller-selected output path and truncates its external target.
4. Export preserves unchanged symlinks, including absolute targets, while import_profile() rejects all link members. This produces successful but unrestorable archives and exposes host paths.
5. The global basename rule for credentials drops safe directories such as workspace/project/credentials/README.md.

Focused suite: 225 passed, 2 skipped; Ruff and git diff --check pass. Each blocker above was also reproduced with a focused temporary-directory probe.

Suggested direction: atomically create output as a new regular file; establish one consistent export/import symlink policy; classify credentials by file/type or canonical path; and either scrub every decodable regular file plus SQLite text fields, reject unsanitizable artifacts, or explicitly remove runtime databases from shareable exports.

I intended this as a request-changes review, but GitHub does not allow this account to submit that review state without explicit repository access.

[keiranhaax]

Updated this PR against current main and addressed all five blockers reported in the latest external review.

Changes now cover:

• SQLite databases are detected by header, captured with WAL-consistent backup, compacted only in the disposable export snapshot to remove deleted credential residue, semantically verified, and rejected when live schema/TEXT/BLOB/UTF-16/URL content is secret-shaped.
• Every staged regular file is inspected regardless of extension; UTF-8 content in .data, .log, extensionless files, and extra_files is scrubbed, while unsafe encoded/binary content fails closed.
• Archive output is created as a temporary regular file and atomically published, so an existing output symlink cannot overwrite its target.
• Profile symlinks are rejected consistently with import policy, while symlinks inside excluded credential trees are neither followed nor archived.
• Credential-directory filtering is path-scoped, preserving safe paths such as workspace/project/credentials/README.md.

The branch was rebased onto current main and the only conflict was additive: current main’s plugin redaction registry and this PR’s conservative secret-hint gate were both preserved.

Validation on exact head 9e37ae14d6c34db9f753c86c0ead8a5c83d440fb:

• 406 profile/redaction tests passed, 2 platform-specific tests skipped
• 120 upstream redaction-registry/redactor tests passed
• 201 focused export/blocker tests passed
• Ruff, Python compilation, and git diff --check passed
• final post-rebase exact-diff review found no blockers

GitHub now reports the PR as mergeable with no conflicts. Ready for maintainer review.

[egilewski]

too large to review safely

This PR changes 1161 production lines before tests and docs. Please split it or add a focused justification if it should stay together.

Signed: GPT-5.6-luna-high in Codex

[keiranhaax]

Thanks for flagging the reviewability concern. I kept the security fix atomic, but reorganized the production code so the boundary can be reviewed in focused sections.

Reviewability update on exact head 08255239ffcb870e914fc40aa24b62468c85de41

• Extracted the cohesive export-security implementation from hermes_cli/profiles.py into hermes_cli/profile_export.py.
• Kept export_profile() as the orchestration entry point in profiles.py.
• Preserved the existing private helper import/monkeypatch surface to avoid compatibility churn.
• Added PR_35601_REVIEW_MAP.md, which maps seven invariants to exact implementation regions and test classes.
• Kept the focused tests in one file because its seven existing classes already follow those same invariant boundaries.

The extraction is mechanical rather than architectural: all 52 moved definitions were checked as AST-equivalent to the previous exact PR head, and the export/import orchestration remained unchanged.

Why the security changes should stay atomic

These protections close different paths through one shareable-archive boundary:

1. path and credential classification
2. staging, extra_files, and symlink policy
3. WAL-consistent SQLite snapshotting
4. SQLite compaction and semantic verification
5. live SQLite secret inspection
6. extension-independent staged-file scrubbing
7. atomic archive publication

Splitting them into independently mergeable fixes would create unsafe intermediate states. For example, a WAL-consistent snapshot can still preserve live credentials without SQLite inspection; generic file scrubbing cannot safely rewrite SQLite; and safe staging can still damage an external target without atomic publication. The focused module and review map make each layer independently reviewable while preserving the complete guarantee.

Local validation on the exact head

• 201 focused export/security tests passed
• 425 export/profile/redaction tests passed, 2 platform-specific tests skipped
• 429 related SQLite/state/backup tests passed, 2 skipped
• all 11 adversarial export probes passed
• real temporary HERMES_HOME exports passed for default and named profiles
• Ruff, Python compilation, import-cycle checks, and git diff --check passed

GitHub currently reports the updated head as mergeable with no conflicts.

[DiscoStew6082]

I found one small change here that seems useful independently of profile export. On current main, strict redaction misses array-style query parameters such as access_token[]=... and access_token%5B%5D=.... This affects both memory context sent for compression and compaction summaries. This PR masks both forms.

Would you be open to extracting that shared-redactor fix and its focused tests into a small standalone PR? I’m happy to help reproduce or review it.

[keiranhaax]

Thanks for the focused suggestion. I extracted the independently useful shared-redactor change into #85762:

• masks literal access_token[]=... and percent-encoded access_token%5B%5D=...
• preserves similarly named public keys such as token_count[] and access_tokenized[]
• covers the shared redactor, compaction-summary boundary, and pre-compression memory-context path
• 268 focused redaction/compression tests pass, with the new regression assertions proven to fail against the pre-fix implementation

I kept the seven interdependent profile-export protections together in this PR. Once the standalone redactor fix lands, this branch can be rebased and the duplicated shared-redactor change removed.

[rcktmonk]

Real-world confirmation from a stopped named profile on Hermes v0.20.5: hermes profile export personal-interests produced a 39,933,458-byte archive with 1,227 members, including personal-interests/mcp-tokens/plaud-official.json, .client.json, and .meta.json. Import was stopped before extraction; the archive was hashed for the local receipt and deleted without upload or sharing.

I also confirmed current main still limits named-profile copy-time exclusion to auth.json and .env. This supports the structural path/token-store boundary in this PR. Separately, the live test found opaque credentials in staged config.yaml are not fully covered here (password_hash is absent from this PR); I am keeping that as a small focused change rather than widening #35601 further.

[rcktmonk]

Follow-up to the real-world reproduction above: I opened focused PR #93995 for the uncovered staged-config.yaml half only. It structurally clears opaque credential fields, keeps exported YAML valid, materializes a staged config symlink before writing, and containment-checks extra_files overlays so they cannot write through leaf/parent symlinks. #35601 remains the broader path/token-store fix; #93995 intentionally does not duplicate its mcp-tokens/ filtering.