fix(distribution): respect distribution_owned allowlist in _copy_dist_payload (#74373) by webtecnica · Pull Request #74414 · NousResearch/hermes-agent

webtecnica · 2026-07-29T22:22:28Z

Summary

_copy_dist_payload() iterated every staged entry with only USER_OWNED_EXCLUDE filtering — the manifest's distribution_owned allowlist was declaration-only. owned_paths() existed on DistributionManifest but was never consulted from the mutation path.

This meant a manifest like:

distribution_owned:
  - SOUL.md

would still install unlisted.txt, .github/, docs/, etc.

Fix

Compute manifest.owned_paths() at the top of _copy_dist_payload() and skip entries not in that set (after the USER_OWNED_EXCLUDE check). owned_paths() already correctly falls back to DEFAULT_DIST_OWNED when no explicit distribution_owned is set, preserving backward compatibility for existing manifests.

Changes

hermes_cli/profile_distribution.py: Added owned = set(manifest.owned_paths()) + if name not in owned: continue in _copy_dist_payload() (the inner for entry loop).
tests/hermes_cli/test_profile_distribution.py:
- test_install_respects_distribution_owned_allowlist — verifies only paths in distribution_owned are copied
- test_install_default_owned_paths_preserved — verifies backward compat when no explicit list is set
- test_update_respects_distribution_owned_allowlist — verifies update path also constrains
- Updated nested-dir tests to explicitly add extra paths to distribution_owned in their manifests

Test results

All 72 tests in test_profile_distribution.py pass.

Closes #74373

…_payload _copy_dist_payload() in profile_distribution.py iterated all staged entries without consulting the manifest's distribution_owned allowlist, so manifests that restricted distribution_owned only had cosmetic effect. Fix: compute manifest.owned_paths() at the top of _copy_dist_payload() and skip entries not in that set, after the USER_OWNED_EXCLUDE check. The owned_paths() method already existed on DistributionManifest and correctly falls back to DEFAULT_DIST_OWNED when no explicit distribution_owned is set, so the new filter preserves backward compatibility for existing manifests. Closes NousResearch#74373

…NousResearch#74339) The root-cause diagnosis in NousResearch#74339 found that _sync_device_code_entry_to_auth_store checked key *presence* (providers.<id> in the profile store) to decide whether a profile resolved its grant from the global root fallback. The function itself called _store_provider_state on every sync, which creates that key. Result: the condition was self-sealing — write-through fired on the first refresh per profile, then permanently disabled itself. Fix --- 1. Use _load_provider_state_with_source (already exposed by hermes_cli/auth.py) instead of _load_provider_state inside each provider branch. The former returns the *source path* the state was resolved from, which is the right signal — not key presence. 2. When the source path is the global root, write-through to root but **skip the _store_provider_state call** that creates a local providers.<id> shadow. Without the shadow key, every subsequent refresh in that profile also resolves from root and again write-throughs. 3. The existing _load_provider_state fallback guarantees that a profile without a local providers.<id> block reads the latest tokens from root, so reads remain correct. Test changes ------------ - test_pool_refresh_writes_through_to_root_when_profile_reads_root: updated assertion — the profile no longer gets a local block when the grant was resolved from root. - New test_pool_refresh_writes_through_to_root_on_every_refresh: parametrized over openai-codex and xai-oauth, drives two consecutive refreshes and verifies root is updated *both* times while the profile stays clean. Fixes: NousResearch#74339

teknium1

Thanks for isolating the unconsulted allowlist; the reported current-main defect is real: hermes_cli/profile_distribution.py:563-590 copies all staged top-level entries except hard exclusions.

Problems

The new fallback at hermes_cli/profile_distribution.py:570 changes omitted-list behavior from copying unexcluded payloads to copying only DEFAULT_DIST_OWNED, despite the stated backward-compatibility goal.
Root-name matching cannot implement the documented nested entries (skills/research/, cron/digest.json) in website/docs/user-guide/profile-distributions.md:254-257.
The PR also contains commit 1b82637437e2, which changes unrelated credential, gateway, and Windows-desktop behavior. Its desktop token at apps/desktop/electron/main.ts:3080 has no updater-side reader.

Suggested changes

Salvage this as a focused, path-aware distribution fix: preserve omitted-list legacy behavior if intended, and test the documented nested-path examples plus target-only sibling preservation.
Split the unrelated commit into separately reviewable changes.

Automated hermes-sweeper review.

teknium1 · 2026-07-30T17:56:33Z

    """
    target.mkdir(parents=True, exist_ok=True)

+    owned = set(manifest.owned_paths())


owned_paths() falls back to DEFAULT_DIST_OWNED for an omitted manifest field, but current main copies all top-level staged entries except hard exclusions. This makes existing manifests silently stop installing arbitrary payload files. Apply the filter only when distribution_owned was explicitly declared, or document and migrate the intentional breaking change.

teknium1 · 2026-07-30T17:56:33Z


        if name in USER_OWNED_EXCLUDE:
            continue
+        if name not in owned:


This compares a root entry name such as skills against the full ownership path. The documented distribution_owned: [skills/research/, cron/digest.json] examples cannot match here. Please make copying relative-path-aware so selected descendants are copied without replacing unowned siblings.

teknium1 · 2026-07-30T17:56:34Z

      env: {
        ...process.env,
        HERMES_HOME,
+        HERMES_WINDOWS_GATEWAY_RESUME_TOKEN: gatewayToken


No updater code in this PR or current main reads HERMES_WINDOWS_GATEWAY_RESUME_TOKEN; update_cmd.py resumes only its locally created token. The desktop-pause token is therefore not handed off on a successful update. Add the receiving path and an end-to-end resume test, or keep this out of the distribution PR.

teknium1 · 2026-07-30T17:56:34Z

+        "remaining_seconds": 300,
+        "error": "previous failure",
+    }
+    runner._session_db = SimpleNamespace(_db=_fake_db)


Production awaits methods on the AsyncSessionDB facade, but this fixture provides only SimpleNamespace(_db=MagicMock()). The new lookup/record calls raise AttributeError and are swallowed by the broad exception, so this does not test persistence and the record assertion cannot pass. Use an async-compatible facade double or invoke the synchronous _db explicitly.

@webtecnica

…ing when omitted Follow-ups to the previous commit (#74414 by @webtecnica, re #74373): - When distribution_owned is OMITTED, restore the legacy contract: every staged entry outside USER_OWNED_EXCLUDE is copied. The cherry-picked filter consulted owned_paths(), which silently narrowed omitted-list distributions to DEFAULT_DIST_OWNED and dropped undeclared payload (extra top-level files/dirs existing distributions legitimately ship). - Make explicit allowlists path-aware so documented nested entries like skills/research/ and cron/digest.json select exactly that subtree/file instead of being dropped by the top-level name comparison. Traversal segments (.., absolute) and USER_OWNED_EXCLUDE roots are still rejected. - Regression tests: omitted-list legacy behavior + nested-path allowlist.

teknium1 · 2026-08-01T06:20:27Z

Merged via salvage PR #75888 (#75888) — your allowlist-enforcement commit was cherry-picked with authorship preserved, plus fix-ups: omitted-list keeps legacy copy-everything behavior, and matching is now path-aware for nested entries. The unrelated credential-pool/electron commit was dropped. Fixes #74373. Thanks!

@webtecnica

…ing when omitted Follow-ups to the previous commit (NousResearch#74414 by @webtecnica, re NousResearch#74373): - When distribution_owned is OMITTED, restore the legacy contract: every staged entry outside USER_OWNED_EXCLUDE is copied. The cherry-picked filter consulted owned_paths(), which silently narrowed omitted-list distributions to DEFAULT_DIST_OWNED and dropped undeclared payload (extra top-level files/dirs existing distributions legitimately ship). - Make explicit allowlists path-aware so documented nested entries like skills/research/ and cron/digest.json select exactly that subtree/file instead of being dropped by the top-level name comparison. Traversal segments (.., absolute) and USER_OWNED_EXCLUDE roots are still rejected. - Regression tests: omitted-list legacy behavior + nested-path allowlist.

@webtecnica

…ing when omitted Follow-ups to the previous commit (NousResearch#74414 by @webtecnica, re NousResearch#74373): - When distribution_owned is OMITTED, restore the legacy contract: every staged entry outside USER_OWNED_EXCLUDE is copied. The cherry-picked filter consulted owned_paths(), which silently narrowed omitted-list distributions to DEFAULT_DIST_OWNED and dropped undeclared payload (extra top-level files/dirs existing distributions legitimately ship). - Make explicit allowlists path-aware so documented nested entries like skills/research/ and cron/digest.json select exactly that subtree/file instead of being dropped by the top-level name comparison. Traversal segments (.., absolute) and USER_OWNED_EXCLUDE roots are still rejected. - Regression tests: omitted-list legacy behavior + nested-path allowlist.

@webtecnica

…ing when omitted Follow-ups to the previous commit (NousResearch#74414 by @webtecnica, re NousResearch#74373): - When distribution_owned is OMITTED, restore the legacy contract: every staged entry outside USER_OWNED_EXCLUDE is copied. The cherry-picked filter consulted owned_paths(), which silently narrowed omitted-list distributions to DEFAULT_DIST_OWNED and dropped undeclared payload (extra top-level files/dirs existing distributions legitimately ship). - Make explicit allowlists path-aware so documented nested entries like skills/research/ and cron/digest.json select exactly that subtree/file instead of being dropped by the top-level name comparison. Traversal segments (.., absolute) and USER_OWNED_EXCLUDE roots are still rejected. - Regression tests: omitted-list legacy behavior + nested-path allowlist.

webtecnica added 2 commits July 29, 2026 19:21

github-actions Bot mentioned this pull request Jul 29, 2026

🦞 OpenClaw 生态日报 2026-07-30 junlinzhao327-oss/big_model_radar#195

Open

alt-glitch mentioned this pull request Jul 30, 2026

fix(profile): respect distribution_owned in _copy_dist_payload #74498

Closed

4 tasks

teknium1 reviewed Jul 30, 2026

View reviewed changes

GottZ mentioned this pull request Jul 30, 2026

distribution_owned does not constrain profile distribution copy/update payload #74373

Closed

alt-glitch mentioned this pull request Jul 31, 2026

fix(profiles): support nested owned paths, correctly distinguish stale distributed files from user additions #75351

Closed

teknium1 mentioned this pull request Aug 1, 2026

fix(gateway): /model off the event loop; distribution allowlist enforced; hindsight env file 0600 #75888

Merged

teknium1 closed this Aug 1, 2026

Navigation Menu

fix(distribution): respect distribution_owned allowlist in _copy_dist_payload (#74373) - #74414

fix(distribution): respect distribution_owned allowlist in _copy_dist_payload (#74373)#74414
webtecnica wants to merge 2 commits into
NousResearch:mainfrom
webtecnica:fix/74373-distribution-owned-payload

webtecnica commented Jul 29, 2026

Uh oh!

teknium1 left a comment

Uh oh!

teknium1 Jul 30, 2026

Uh oh!

teknium1 Jul 30, 2026

Uh oh!

teknium1 Jul 30, 2026

Uh oh!

teknium1 Jul 30, 2026

Uh oh!

teknium1 commented Aug 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

webtecnica commented Jul 29, 2026

Summary

Fix

Changes

Test results

Uh oh!

teknium1 left a comment

Choose a reason for hiding this comment

Problems

Suggested changes

Uh oh!

teknium1 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

teknium1 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

teknium1 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

teknium1 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

teknium1 commented Aug 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants