Skip to content

fix(slack): warn when slack_tokens.json is group/world-readable - #60009

Closed
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/slack-token-file-permission-check
Closed

fix(slack): warn when slack_tokens.json is group/world-readable#60009
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/slack-token-file-permission-check

Conversation

@AlexFucuson9

@AlexFucuson9 AlexFucuson9 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Warn at startup when slack_tokens.json is group- or world-readable.

Problem

plugins/platforms/slack/adapter.py:985 — the OAuth multi-workspace token file contains plaintext bot tokens for all saved Slack workspaces. Unlike the Google Chat adapter (google_chat/oauth.py:329) which explicitly sets 0o600 permissions when writing credential files, the Slack token file has no permission enforcement. With a default umask of 022, the file is world-readable, exposing all saved workspace tokens to any local user.

Fix

Check file permissions when reading slack_tokens.json. If the file is group-readable (S_IRGRP) or world-readable (S_IROTH), emit a logger.warning with the current mode and remediation command (chmod 600).

This follows the defense-in-depth pattern: don't refuse to load (that would break existing setups), but make the misconfiguration visible so operators can fix it.

Testing

1-file change. The warning is a log-only side effect that doesn't affect existing functionality.

@AlexFucuson9
fix(slack): warn when slack_tokens.json is group/world-readable
6744f1c 
The OAuth multi-workspace token file contains plaintext bot tokens for
all saved Slack workspaces. Unlike the Google Chat adapter which sets
0o600 when writing credentials, the Slack token file has no permission
enforcement — a default umask 022 makes it world-readable.

Fix: check file permissions on read and emit a warning log with remediation
instructions if the file is group- or world-readable.
@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/plugins Plugin system and bundled plugins platform/slack Slack app adapter sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data needs-repro Bug needs reproduction steps P3 Low — cosmetic, nice to have labels Jul 7, 2026
@teknium1

teknium1 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused Slack credential-hardening change. The current loader reads slack_tokens.json directly at plugins/platforms/slack/adapter.py:997-1000, so the warning addresses a real gap and the target hunk remains current.

Problems

  • There is no regression test for the new mode check. The existing Slack warning tests demonstrate the project’s caplog pattern at tests/gateway/test_slack_user_token_warning.py:74-120; add coverage for insecure and owner-only token-file modes.
  • slack_tokens.json is documented to contain plaintext bot tokens (website/docs/user-guide/messaging/slack.md:576-595) but is not in the gateway root credential denylist at gateway/platforms/base.py:1174-1192. Please include it and add the corresponding denylist regression test, following tests/gateway/test_platform_base.py:1087-1127.

Suggested changes

  • Test both warning and no-warning mode paths against a temporary HERMES_HOME.
  • Deny slack_tokens.json from gateway media delivery alongside the other root credential files.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 15, 2026
teknium1 added a commit that referenced this pull request Jul 23, 2026
@teknium1
harden(slack): CDN-allowlist inbound file URLs, DNS-pin token downloa…
1508858 
…ds, widen token-file perms warning

Follow-up hardening on top of the C14 cherry-picks (#57860/#44026/#66742/#60009):

- Slack file downloads (_download_slack_file/_download_slack_file_bytes)
  now require an https URL on a Slack CDN host (files.slack.com,
  *.slack.com Enterprise Grid, *.slack-files.com legacy shares) before
  attaching the bot token. url_private/url_private_download only ever
  point at the Slack CDN, so a forged file object from a malicious
  workspace app or compromised event stream pointing the Bearer-token
  download at an arbitrary PUBLIC host (token exfiltration) is now
  refused — a hole #44026's generic private-IP SSRF check alone could
  not close.
- The same two download paths now use create_ssrf_safe_async_client
  (from #57860) so the preflight-validated hostname is resolved once,
  validated, and dialed by IP — closing the DNS-rebinding TOCTOU window
  for the token-bearing inbound fetches as well.
- #60009's slack_tokens.json permission warning is generalized into
  utils.warn_if_credential_file_broadly_readable() (POSIX-only,
  fail-quiet) and wired into the other read path with the same gap:
  google_chat's load_user_credentials(). google_chat already writes
  0o600 via _write_private_json; the read-time warning covers
  hand-provisioned/legacy files. Nothing in-repo writes
  slack_tokens.json (user/OAuth-provisioned), so there is no write
  path to chmod for Slack.

Security tests both directions: non-CDN/lookalike/http URLs and
connect-time DNS rebinds are blocked before any TCP connect; real
files.slack.com, Enterprise Grid, and slack-files.com URLs still reach
the network layer; 0o600 files stay silent while 0o644/0o640 warn with
a chmod hint. A/B: all 10 new download-guard tests fail with the
hardening reverted and pass with it applied.
@teknium1 teknium1 mentioned this pull request Jul 23, 2026
Merged
teknium1 added a commit that referenced this pull request Jul 23, 2026
@teknium1
harden(slack): CDN-allowlist inbound file URLs, DNS-pin token downloa…
8e4b5d8 
…ds, widen token-file perms warning

Follow-up hardening on top of the C14 cherry-picks (#57860/#44026/#66742/#60009):

- Slack file downloads (_download_slack_file/_download_slack_file_bytes)
  now require an https URL on a Slack CDN host (files.slack.com,
  *.slack.com Enterprise Grid, *.slack-files.com legacy shares) before
  attaching the bot token. url_private/url_private_download only ever
  point at the Slack CDN, so a forged file object from a malicious
  workspace app or compromised event stream pointing the Bearer-token
  download at an arbitrary PUBLIC host (token exfiltration) is now
  refused — a hole #44026's generic private-IP SSRF check alone could
  not close.
- The same two download paths now use create_ssrf_safe_async_client
  (from #57860) so the preflight-validated hostname is resolved once,
  validated, and dialed by IP — closing the DNS-rebinding TOCTOU window
  for the token-bearing inbound fetches as well.
- #60009's slack_tokens.json permission warning is generalized into
  utils.warn_if_credential_file_broadly_readable() (POSIX-only,
  fail-quiet) and wired into the other read path with the same gap:
  google_chat's load_user_credentials(). google_chat already writes
  0o600 via _write_private_json; the read-time warning covers
  hand-provisioned/legacy files. Nothing in-repo writes
  slack_tokens.json (user/OAuth-provisioned), so there is no write
  path to chmod for Slack.

Security tests both directions: non-CDN/lookalike/http URLs and
connect-time DNS rebinds are blocked before any TCP connect; real
files.slack.com, Enterprise Grid, and slack-files.com URLs still reach
the network layer; 0o600 files stay silent while 0o644/0o640 warn with
a chmod hint. A/B: all 10 new download-guard tests fail with the
hardening reverted and pass with it applied.
@teknium1

teknium1 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Merged via #70193 — your commit was cherry-picked/reapplied onto current main with your authorship preserved in git history: your token-file permission warning was cherry-picked and widened to chmod-on-write for the file class.

Thanks for the contribution!

@teknium1 teknium1 closed this Jul 23, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
@teknium1
harden(slack): CDN-allowlist inbound file URLs, DNS-pin token downloa…
8a1f90f 
…ds, widen token-file perms warning

Follow-up hardening on top of the C14 cherry-picks (NousResearch#57860/NousResearch#44026/NousResearch#66742/NousResearch#60009):

- Slack file downloads (_download_slack_file/_download_slack_file_bytes)
  now require an https URL on a Slack CDN host (files.slack.com,
  *.slack.com Enterprise Grid, *.slack-files.com legacy shares) before
  attaching the bot token. url_private/url_private_download only ever
  point at the Slack CDN, so a forged file object from a malicious
  workspace app or compromised event stream pointing the Bearer-token
  download at an arbitrary PUBLIC host (token exfiltration) is now
  refused — a hole NousResearch#44026's generic private-IP SSRF check alone could
  not close.
- The same two download paths now use create_ssrf_safe_async_client
  (from NousResearch#57860) so the preflight-validated hostname is resolved once,
  validated, and dialed by IP — closing the DNS-rebinding TOCTOU window
  for the token-bearing inbound fetches as well.
- NousResearch#60009's slack_tokens.json permission warning is generalized into
  utils.warn_if_credential_file_broadly_readable() (POSIX-only,
  fail-quiet) and wired into the other read path with the same gap:
  google_chat's load_user_credentials(). google_chat already writes
  0o600 via _write_private_json; the read-time warning covers
  hand-provisioned/legacy files. Nothing in-repo writes
  slack_tokens.json (user/OAuth-provisioned), so there is no write
  path to chmod for Slack.

Security tests both directions: non-CDN/lookalike/http URLs and
connect-time DNS rebinds are blocked before any TCP connect; real
files.slack.com, Enterprise Grid, and slack-files.com URLs still reach
the network layer; 0o600 files stay silent while 0o644/0o640 warn with
a chmod hint. A/B: all 10 new download-guard tests fail with the
hardening reverted and pass with it applied.
prmartinow pushed a commit to prmartinow/hermes-agent that referenced this pull request Aug 26, 2026
@teknium1
harden(slack): CDN-allowlist inbound file URLs, DNS-pin token downloa…
6f33d4d 
…ds, widen token-file perms warning

Follow-up hardening on top of the C14 cherry-picks (NousResearch#57860/NousResearch#44026/NousResearch#66742/NousResearch#60009):

- Slack file downloads (_download_slack_file/_download_slack_file_bytes)
  now require an https URL on a Slack CDN host (files.slack.com,
  *.slack.com Enterprise Grid, *.slack-files.com legacy shares) before
  attaching the bot token. url_private/url_private_download only ever
  point at the Slack CDN, so a forged file object from a malicious
  workspace app or compromised event stream pointing the Bearer-token
  download at an arbitrary PUBLIC host (token exfiltration) is now
  refused — a hole NousResearch#44026's generic private-IP SSRF check alone could
  not close.
- The same two download paths now use create_ssrf_safe_async_client
  (from NousResearch#57860) so the preflight-validated hostname is resolved once,
  validated, and dialed by IP — closing the DNS-rebinding TOCTOU window
  for the token-bearing inbound fetches as well.
- NousResearch#60009's slack_tokens.json permission warning is generalized into
  utils.warn_if_credential_file_broadly_readable() (POSIX-only,
  fail-quiet) and wired into the other read path with the same gap:
  google_chat's load_user_credentials(). google_chat already writes
  0o600 via _write_private_json; the read-time warning covers
  hand-provisioned/legacy files. Nothing in-repo writes
  slack_tokens.json (user/OAuth-provisioned), so there is no write
  path to chmod for Slack.

Security tests both directions: non-CDN/lookalike/http URLs and
connect-time DNS rebinds are blocked before any TCP connect; real
files.slack.com, Enterprise Grid, and slack-files.com URLs still reach
the network layer; 0o600 files stay silent while 0o644/0o640 warn with
a chmod hint. A/B: all 10 new download-guard tests fail with the
hardening reverted and pass with it applied.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/plugins Plugin system and bundled plugins needs-repro Bug needs reproduction steps P3 Low — cosmetic, nice to have platform/slack Slack app adapter sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AlexFucuson9 @teknium1 @alt-glitch