Skip to content

security(environments): protect modal/singularity snapshot stores with owner-only perms - #60259

Closed
isheng-eqi wants to merge 8 commits into
NousResearch:mainfrom
isheng-eqi:fix/env-snapshot-store-profile-aware
Closed

security(environments): protect modal/singularity snapshot stores with owner-only perms#60259
isheng-eqi wants to merge 8 commits into
NousResearch:mainfrom
isheng-eqi:fix/env-snapshot-store-profile-aware

Conversation

@isheng-eqi

@isheng-eqi isheng-eqi commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Two fixes: (1) _save_json_store() writes snapshot data without umask — add umask 077 like #60199. (2) _SNAPSHOT_STORE module-level constants in modal/singularity stale in multi-profile runtimes (#40677).

Related Issue

Fixes sibling paths of #60199 and #60180.

Type of Change

  • Bug fix
  • Security fix

Changes Made

  • tools/environments/base.py: Add umask guard
  • tools/environments/modal.py: Replace constant with _snapshot_store_path()
  • tools/environments/singularity.py: Replace constant with _snapshot_store_path()

The gateway already strips stale dangerous-confirmation text from
replayed history (NousResearch#60110, NousResearch#60117 — fix for NousResearch#59607). The TUI gateway
has two session resume paths that sanitize replay history but do not
strip stale dangerous confirmations, leaving the same vulnerability
unaddressed in TUI sessions.

Add strip_stale_dangerous_confirmations() after sanitize_replay_history()
in both TUI resume paths (resume_session and the WebUI resume path).

Refs: NousResearch#59607, NousResearch#60110, NousResearch#60117
The recent Discord resource-bounding pass (NousResearch#60122, NousResearch#60112, NousResearch#60113)
added limits for REST JSON/error response bodies and component label
UTF-16 lengths. Four HTTP response reads for image/animation/attachment
downloads were left unbounded — an oversized response from a CDN or
external URL could OOM the bot.

Add _DISCORD_IMAGE_DOWNLOAD_MAX_BYTES (50 MB) and
_DISCORD_ATTACHMENT_DOWNLOAD_MAX_BYTES (100 MB) constants, a shared
_read_response_bytes_bounded() helper, and apply bounds to:
- Batch image download (adapter.py ~2467)
- Single image download (adapter.py ~3555)
- Animation/GIF download (adapter.py ~3634)
- Attachment download (adapter.py ~5799)

Refs: NousResearch#60122, NousResearch#60112
The profile-aware skills resolution fix (NousResearch#60180) added _skills_dir() to
tools/skills_tool.py and tools/skill_manager_tool.py so that long-lived
multi-profile runtimes always resolve the live profile-scoped HERMES_HOME.
agent/skill_utils.py still referenced the module-level SKILLS_DIR constant,
which would return stale import-time paths in the same long-lived runtimes.

Replace _skills_tool.SKILLS_DIR with _skills_tool._skills_dir() in the
normalize_skill_identifier() path, matching the pattern established in
NousResearch#60180. Tests that patch SKILLS_DIR are unaffected — _skills_dir() falls
back to the patched value when it differs from _SKILLS_DIR_AT_IMPORT.

Refs: NousResearch#60180, NousResearch#40677
… runtimes

Module-level _AUTH_JSON_PATH captures get_hermes_home() at import time.
In long-lived multi-profile runtimes (Dashboard/TUI/Desktop backend),
this path becomes stale when the active profile changes, causing auth
token reads from the wrong profile (NousResearch#40677).

Add _auth_json_path() that resolves from the live HERMES_HOME, matching
the pattern established by _skills_dir() in NousResearch#60180.

Refs: NousResearch#60180, NousResearch#40677
…n profile scope

After NousResearch#60180, _skills_dir() resolves SKILLS_DIR at call time via the
live get_hermes_home(). The manual monkeypatching of
_skills_tool.SKILLS_DIR/HERMES_HOME and _skill_mgr.SKILLS_DIR/HERMES_HOME
in _profile_scope() is no longer necessary — set_hermes_home_override()
alone suffices. Remove the redundant patching, drop the unused module
imports, and update stale docstrings in both _profile_scope() and
_config_profile_scope().

Refs: NousResearch#60180, NousResearch#40677
…-lived runtimes

get_skill_commands() caches scanned skills in module-level _skill_commands
and only invalidates when the active platform changes (NousResearch#14536). In
long-lived multi-profile runtimes (Dashboard/TUI/Desktop backend), the
cache is not invalidated when the active profile changes, causing the
wrong profile's skills to appear in the slash-command map.

Add _skill_commands_skills_dir tracking alongside the existing platform
check. On each get_skill_commands() call, compare the cached skills_dir
against the current _skills_dir() — if they differ (profile change),
rescan. Also update scan_skill_commands() to use _skills_dir() (call-time
resolution, NousResearch#60180) instead of the module-level SKILLS_DIR constant.

Refs: NousResearch#60180, NousResearch#40677, NousResearch#14536
…er-only perms and profile-awareness

Two fixes completing the NousResearch#60199 security hardening + NousResearch#60180 profile-awareness:

1. Security: _save_json_store() writes snapshot data (which may contain
   env-carried secrets) without umask protection. Add os.umask(0o077)
   guard matching the env snapshot write fix in NousResearch#60199.

2. Profile-awareness: _SNAPSHOT_STORE in modal.py and singularity.py
   captured get_hermes_home() at import time. Replace with call-time
   _snapshot_store_path() resolvers so the live profile-scoped
   HERMES_HOME is always respected in long-lived multi-profile runtimes
   (NousResearch#40677).

Refs: NousResearch#60199, NousResearch#60180, NousResearch#40677
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) backend/modal Modal.com cloud execution backend/singularity Singularity container execution sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P3 Low — cosmetic, nice to have labels Jul 7, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Part of the profile-awareness stacked chain (#60180 base merged → #60231/#60241/#60254/#60257/#60259). Note: this PR's diff carries 6 files (auxiliary_client, skill_commands, skill_utils, web_server, discord adapter, tui_gateway/server) that are byte-identical to the earlier stack siblings — the unique change is the tools/environments/{base,modal,singularity}.py umask + call-time snapshot-path fix. Related: #60257 (sibling), #60180 (base).

@isheng-eqi

Copy link
Copy Markdown
Contributor Author

Superseded by #60317 (snapshot perms + tests) and #60264 (xAI OAuth). All content is now covered by smaller, focused PRs.

@isheng-eqi isheng-eqi closed this Jul 7, 2026
@isheng-eqi isheng-eqi changed the title fix(environments): protect modal/singularity snapshot stores with owner-only perms and profile-awareness security(environments): protect modal/singularity snapshot stores with owner-only perms Jul 7, 2026
@isheng-eqi isheng-eqi reopened this Jul 7, 2026
@isheng-eqi

Copy link
Copy Markdown
Contributor Author

Branch contaminated — rebuilding with clean base.

@isheng-eqi isheng-eqi closed this Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend/modal Modal.com cloud execution backend/singularity Singularity container execution comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants