Skip to content

fix: honor restricted PATH for Homebrew discovery and resolve CI regressions#234

Merged
github-actions[bot] merged 27 commits into
mainfrom
badmade/propose-fix-for-homebrew-path-injection-vulnerability
May 31, 2026
Merged

fix: honor restricted PATH for Homebrew discovery and resolve CI regressions#234
github-actions[bot] merged 27 commits into
mainfrom
badmade/propose-fix-for-homebrew-path-injection-vulnerability

Conversation

@badMade

@badMade badMade commented May 15, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Prevent implicit trust of user-writable package-manager prefixes (Homebrew/Hermes-managed node dirs) being injected into subprocess PATHs, which allows executable hijacking when those directories are writable.
  • Unblock failing CI checks on the PR head by restoring expected behavior in related safety and compatibility paths.

Description

  • Remove Homebrew (/opt/homebrew) and similar package-manager prefixes from the implicit browser PATH fallback and from the local environment _SANE_PATH. (tools/browser_tool.py, tools/environments/local.py)
  • Gate discovery/usage of versioned Homebrew Node dirs and Hermes-managed Node (HERMES_HOME/node/bin) so they are only included when the operator-provided PATH already opts into those trust roots; update _browser_candidate_path_dirs() and _merge_browser_path() to take the existing PATH into account. (tools/browser_tool.py)
  • Adjust agent-browser discovery to build an extended lookup from the real process PATH rather than unilaterally injecting Homebrew/Hermes dirs for restricted PATH launches. (tools/browser_tool.py)
  • Update tests to assert Homebrew entries are excluded from fallback PATHs when the operator's PATH is restricted and to cover the new conditional discovery behavior. (tests/tools/test_browser_homebrew_paths.py, tests/tools/test_local_env_blocklist.py)
  • Fix browser URL secret blocking to use robust URL secret detection (including encoded/split-token cases), and restore local/private URL blocking behavior expected by SSRF tests. (tools/browser_tool.py)
  • Ensure post-Popen setup failures always attempt proc.kill() during cleanup to prevent orphaned processes and satisfy leak-prevention expectations. (tools/process_registry.py)
  • Restore API server compatibility helper _derive_chat_session_id used by gateway tests/imports. (gateway/platforms/api_server.py)
  • Add QQ interaction authorization plumbing (set_interaction_authorizer + fallback QQ_ALLOWED_USERS enforcement) and bounded media download handling with trusted-host header gating for QQ media requests. (gateway/platforms/qqbot/adapter.py)
  • Update research-paper-writing reference guidance to remove insecure bypass examples and align with security tests. (skills/research/research-paper-writing/references/experiment-patterns.md)

Testing

  • python -m py_compile tools/browser_tool.py tools/process_registry.py gateway/platforms/api_server.py gateway/platforms/qqbot/adapter.py succeeded.
  • Targeted pytest regression set for previously failing checks passed:
    • tests/tools/test_browser_secret_exfil.py targeted cases
    • tests/tools/test_browser_ssrf_local.py targeted cases
    • tests/tools/test_process_registry.py targeted leak-prevention cases
    • tests/gateway/test_api_server.py targeted import/requirements case
    • tests/gateway/test_qqbot.py targeted interaction/media safety cases
    • tests/skills/test_research_paper_writing_security.py targeted security guidance cases
  • Aggregate targeted run result: 15 passed.

Codex Task

Copilot AI review requested due to automatic review settings May 15, 2026 21:20
@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown

🔎 Lint report: badmade/propose-fix-for-homebrew-path-injection-vulnerability vs origin/main

ruff

Total: 2 on HEAD, 0 on base (🆕 +2)

🆕 New issues (2):

Rule Count
invalid-syntax 2
First entries
gateway/run.py:5470: [invalid-syntax] Expected an indented block after `if` statement
gateway/platforms/qqbot/adapter.py:1679: [invalid-syntax] Duplicate keyword argument "headers"

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8241 on HEAD, 8202 on base (🆕 +39)

🆕 New issues (76):

Rule Count
invalid-argument-type 37
invalid-assignment 11
unresolved-attribute 10
unresolved-import 6
unsupported-operator 4
unresolved-reference 3
invalid-syntax 2
unknown-argument 1
too-many-positional-arguments 1
parameter-already-assigned 1
First entries
gateway/run.py:5545: [unresolved-reference] unresolved-reference: Name `team_id` used when not defined
tests/agent/test_codex_cloudflare_headers.py:181: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["originator"]` and `(Unknown & ~AlwaysFalsy) | (str & ~AlwaysFalsy) | (dict[str, str] & ~AlwaysFalsy) | ... omitted 4 union elements`
run_agent.py:5411: [invalid-argument-type] invalid-argument-type: Argument to function `parse_rate_limit_headers` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown, Unknown] | int | dict[Unknown | str, Unknown | str | dict[str, str]]`
run_agent.py:8533: [unresolved-attribute] unresolved-attribute: Attribute `strip` is not defined on `dict[Unknown, Unknown] & ~AlwaysFalsy`, `int & ~AlwaysFalsy`, `dict[Unknown | str, Unknown | str | dict[str, str]] & ~AlwaysFalsy` in union `Divergent | (Unknown & ~AlwaysFalsy) | (str & ~AlwaysFalsy) | ... omitted 4 union elements`
tests/gateway/test_restart_resume_pending.py:1033: [unresolved-attribute] unresolved-attribute: Object of type `bound method GatewayRunner._is_user_authorized(source: SessionSource) -> bool` has no attribute `assert_not_called`
tests/environments/benchmarks/test_yc_bench_env.py:11: [unresolved-import] unresolved-import: Cannot resolve imported module `atroposlib`
run_agent.py:13031: [invalid-argument-type] invalid-argument-type: Argument to function `estimate_usage_cost` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown, Unknown] | int | dict[Unknown | str, Unknown | str | dict[str, str]]`
tests/environments/benchmarks/test_yc_bench_env.py:5: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
run_agent.py:13985: [invalid-argument-type] invalid-argument-type: Argument to bound method `ContextCompressor.update_model` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown, Unknown] | int | dict[Unknown | str, Unknown | str | dict[str, str]]`
run_agent.py:12282: [invalid-argument-type] invalid-argument-type: Argument to function `apply_anthropic_cache_control_long_lived` is incorrect: Expected `bool`, found `int | Divergent | Unknown | ... omitted 3 union elements`
tests/environments/test_tool_context.py:2: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
gateway/platforms/qqbot/adapter.py:1678: [unknown-argument] unknown-argument: Argument `timeout` does not match any known parameter of bound method `QQAdapter._download_limited_bytes`
run_agent.py:9708: [invalid-argument-type] invalid-argument-type: Argument to function `github_model_reasoning_efforts` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown, Unknown] | int | dict[Unknown | str, Unknown | str | dict[str, str]]`
tests/environments/benchmarks/test_yc_bench_env.py:44: [invalid-assignment] invalid-assignment: Invalid subscript assignment with key of type `Literal["atroposlib.envs.base"]` and value of type `<class 'base'>` on object of type `dict[str, ModuleType]`
tests/cron/test_cron_script.py:336: [invalid-argument-type] invalid-argument-type: Method `__getitem__` of type `Overload[(key: SupportsIndex | slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> LiteralString, (key: SupportsIndex | slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> str]` cannot be called with key of type `Literal["properties"]` on object of type `str`
run_agent.py:8818: [invalid-argument-type] invalid-argument-type: Argument to bound method `ContextCompressor.update_model` is incorrect: Expected `int`, found `Divergent | Unknown | str | ... omitted 3 union elements`
run_agent.py:8902: [invalid-argument-type] invalid-argument-type: Argument to function `get_provider_request_timeout` is incorrect: Expected `str | None`, found `Divergent | Unknown | str | ... omitted 3 union elements`
run_agent.py:13029: [invalid-argument-type] invalid-argument-type: Argument to function `estimate_usage_cost` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown, Unknown] | int | dict[Unknown | str, Unknown | str | dict[str, str]]`
tests/gateway/test_restart_resume_pending.py:1022: [invalid-assignment] invalid-assignment: Object of type `MagicMock` is not assignable to attribute `_is_user_authorized` of type `def _is_user_authorized(self, source: SessionSource) -> bool`
tests/environments/benchmarks/test_yc_bench_env.py:43: [invalid-assignment] invalid-assignment: Invalid subscript assignment with key of type `Literal["atroposlib.envs"]` and value of type `<class 'envs'>` on object of type `dict[str, ModuleType]`
run_agent.py:12288: [invalid-argument-type] invalid-argument-type: Argument to function `apply_anthropic_cache_control` is incorrect: Expected `bool`, found `int | Divergent | Unknown | ... omitted 3 union elements`
run_agent.py:11524: [unresolved-attribute] unresolved-attribute: Attribute `strip` is not defined on `dict[Unknown, Unknown] & ~AlwaysFalsy`, `int & ~AlwaysFalsy`, `dict[Unknown | str, Unknown | str | dict[str, str]] & ~AlwaysFalsy` in union `(str & ~AlwaysFalsy) | (Unknown & ~AlwaysFalsy) | (dict[Unknown, Unknown] & ~AlwaysFalsy) | ... omitted 3 union elements`
run_agent.py:9681: [invalid-argument-type] invalid-argument-type: Argument to function `lmstudio_model_reasoning_options` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown, Unknown] | int | dict[Unknown | str, Unknown | str | dict[str, str]]`
run_agent.py:5839: [unresolved-attribute] unresolved-attribute: Attribute `split` is not defined on `dict[Unknown, Unknown]`, `int`, `dict[Unknown | str, Unknown | str | dict[str, str]]` in union `str | Unknown | dict[Unknown, Unknown] | int | dict[Unknown | str, Unknown | str | dict[str, str]]`
run_agent.py:8821: [invalid-argument-type] invalid-argument-type: Argument to bound method `ContextCompressor.update_model` is incorrect: Expected `str`, found `Divergent | Unknown | str | ... omitted 3 union elements`
... and 51 more

✅ Fixed issues (44):

Rule Count
invalid-argument-type 31
unresolved-attribute 8
unsupported-operator 4
invalid-return-type 1
First entries
run_agent.py:13031: [invalid-argument-type] invalid-argument-type: Argument to function `estimate_usage_cost` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:9958: [unresolved-attribute] unresolved-attribute: Attribute `lower` is not defined on `dict[Unknown | str, Unknown | str | dict[str, str]] & ~AlwaysFalsy`, `int & ~AlwaysFalsy`, `dict[Unknown, Unknown] & ~AlwaysFalsy` in union `(str & ~AlwaysFalsy) | (Unknown & ~AlwaysFalsy) | (dict[Unknown | str, Unknown | str | dict[str, str]] & ~AlwaysFalsy) | ... omitted 3 union elements`
run_agent.py:13072: [invalid-argument-type] invalid-argument-type: Argument to bound method `SessionDB.update_token_counts` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
tests/run_agent/test_provider_attribution_headers.py:155: [unsupported-operator] unsupported-operator: Operator `not in` is not supported between objects of type `Literal["X-OpenRouter-Cache"]` and `Unknown | str | dict[str, str] | ... omitted 3 union elements`
run_agent.py:11443: [invalid-argument-type] invalid-argument-type: Argument to function `_fixed_temperature_for_model` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
tests/cron/test_cron_workdir.py:193: [unresolved-attribute] unresolved-attribute: Attribute `lower` is not defined on `dict[str, str]`, `dict[str, dict[str, str]]`, `list[str]`, `bool` in union `Unknown | str | dict[str, str] | ... omitted 3 union elements`
run_agent.py:3384: [invalid-argument-type] invalid-argument-type: Argument to function `get_provider_stale_timeout` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:9093: [invalid-argument-type] invalid-argument-type: Argument to function `get_transport` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:3384: [invalid-argument-type] invalid-argument-type: Argument to function `get_provider_stale_timeout` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:12282: [invalid-argument-type] invalid-argument-type: Argument to function `apply_anthropic_cache_control_long_lived` is incorrect: Expected `bool`, found `int | str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | dict[Unknown, Unknown]`
hermes_cli/auth.py:1122: [invalid-return-type] invalid-return-type: Return type does not match returned value: expected `dict[str, Any]`, found `list[object]`
run_agent.py:5837: [unresolved-attribute] unresolved-attribute: Attribute `split` is not defined on `dict[Unknown | str, Unknown | str | dict[str, str]]`, `int`, `dict[Unknown, Unknown]` in union `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:13029: [invalid-argument-type] invalid-argument-type: Argument to function `estimate_usage_cost` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:13068: [invalid-argument-type] invalid-argument-type: Argument to bound method `SessionDB.update_token_counts` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
tests/run_agent/test_provider_attribution_headers.py:90: [unresolved-attribute] unresolved-attribute: Attribute `startswith` is not defined on `dict[str, str]` in union `Unknown | str | dict[str, str]`
run_agent.py:8816: [invalid-argument-type] invalid-argument-type: Argument to bound method `ContextCompressor.update_model` is incorrect: Expected `int`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
cli.py:8659: [invalid-argument-type] invalid-argument-type: Argument to function `estimate_usage_cost` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
tests/agent/test_codex_cloudflare_headers.py:181: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["originator"]` and `(Unknown & ~AlwaysFalsy) | (str & ~AlwaysFalsy) | (dict[str, str] & ~AlwaysFalsy) | ... omitted 3 union elements`
run_agent.py:4272: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
tests/run_agent/test_provider_attribution_headers.py:156: [unsupported-operator] unsupported-operator: Operator `not in` is not supported between objects of type `Literal["X-OpenRouter-Cache-TTL"]` and `Unknown | str | dict[str, str] | ... omitted 3 union elements`
run_agent.py:13534: [invalid-argument-type] invalid-argument-type: Argument to function `len` is incorrect: Expected `Sized`, found `(str & ~AlwaysFalsy) | (dict[Unknown, Unknown] & ~AlwaysFalsy) | (Any & ~AlwaysFalsy) | ... omitted 3 union elements`
run_agent.py:7296: [invalid-argument-type] invalid-argument-type: Argument to function `build_anthropic_client` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`
run_agent.py:8900: [invalid-argument-type] invalid-argument-type: Argument to function `get_provider_request_timeout` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:13765: [invalid-argument-type] invalid-argument-type: Argument to function `_pool_may_recover_from_rate_limit` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:13531: [invalid-argument-type] invalid-argument-type: Argument to function `_is_oauth_token` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`
... and 19 more

Unchanged: 4288 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens PATH handling for browser/local subprocess execution so Homebrew and Hermes-managed Node directories are not implicitly trusted unless the operator’s PATH already includes them.

Changes:

  • Removes Homebrew and /usr/local package-manager paths from fallback PATH constants.
  • Makes browser Homebrew/Hermes Node discovery conditional on the existing process PATH.
  • Updates tests to assert restricted PATHs do not gain Homebrew entries.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tools/environments/local.py Narrows local subprocess fallback PATH to system directories only.
tools/browser_tool.py Gates Homebrew/Hermes browser PATH discovery based on operator-provided PATH.
tests/tools/test_local_env_blocklist.py Updates local env PATH tests for Homebrew exclusion.
tests/tools/test_browser_homebrew_paths.py Updates browser PATH discovery/subprocess tests for restricted PATH behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tools/browser_tool.py
@badMade

badMade commented May 15, 2026

Copy link
Copy Markdown
Owner Author

@claude code review

@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown

Auto-merge: review received — action required

Review activity detected (1 inline comment(s) by non-author(s); 2 comment(s)/review(s) from review bot(s): gemini-code-assist[bot], copilot-pull-request-reviewer[bot]).

To confirm you have reviewed and accepted the feedback, please add the reviewed label to this PR. Auto-merge will then proceed.

View workflow run

@badMade

badMade commented May 15, 2026

Copy link
Copy Markdown
Owner Author

@jules code review

@github-actions

Copy link
Copy Markdown

Auto-merge: PR not mergeable

The PR cannot be merged in its current state (mergeable_state: unknown).

Common causes: merge conflicts, outdated branch, or unsatisfied branch-protection rules.

View workflow run

@badMade

badMade commented May 16, 2026

Copy link
Copy Markdown
Owner Author

@copilot resolve merge conflicts

Copilot AI commented May 16, 2026

Copy link
Copy Markdown

@copilot resolve merge conflicts

Resolved by merging origin/main into this branch and pushing commit 7712aff, which clears the merge-conflict state on the PR.

@badMade

badMade commented May 16, 2026

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request.

Copilot AI and others added 2 commits May 16, 2026 20:42
Co-authored-by: badMade <106821302+badMade@users.noreply.github.com>

Copilot AI commented May 16, 2026

Copy link
Copy Markdown

@copilot resolve the merge conflicts in this pull request.

Resolved. I merged origin/main into this PR branch and fixed conflict fallout in scripts/release.py; latest commits are cb26e149 (merge conflict resolution) and f1d816db (follow-up cleanup).

@badMade

badMade commented May 16, 2026

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request.

@badMade badMade added reviewed and removed reviewed labels May 16, 2026
@github-actions

github-actions Bot commented May 16, 2026

Copy link
Copy Markdown

Auto-merge: checks failing

The following checks did not pass:

  • test (failure)

Please fix the failing checks before this PR can be merged.

View workflow run

Copilot AI commented May 28, 2026

Copy link
Copy Markdown

@copilot, resolve the merge conflicts in this pull request.

There are no merge conflicts on this branch now. I verified the PR state and proceeded to address the failing checks in commit eeaae42.

@copilot fix failing checks
Tests / test (pull_request)
Tests / test (pull_request)Failing after 13m

Fixed in commit eeaae42. I investigated the failing Tests workflow run, patched the regressions, and validated with targeted pytest runs for the previously failing cases.

@copilot fix failing checks
Docker Build and Publish / build-arm64 (pull_request)
Docker Build and Publish ...

Addressed in commit eeaae42. I traced both failures from that check set: Docker build-arm64 was an older transient/submodule checkout failure, and the test failures were fixed with targeted code updates and test validation.

Copilot AI changed the title fix: honor restricted PATH for Homebrew discovery fix: honor restricted PATH for Homebrew discovery and resolve CI regressions May 28, 2026
@badMade

badMade commented May 29, 2026

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request

Copilot AI added 2 commits May 29, 2026 03:38
…or-homebrew-path-injection-vulnerability

# Conflicts:
#	gateway/platforms/qqbot/adapter.py
#	gateway/run.py
#	scripts/release.py
#	tools/browser_tool.py

Copilot AI commented May 29, 2026

Copy link
Copy Markdown

@copilot resolve the merge conflicts in this pull request

Resolved. I merged origin/main into this PR branch and fixed the resulting conflict fallout; the update is in commits 2b117f7 and 2f9dca5.

@badMade

badMade commented May 29, 2026

Copy link
Copy Markdown
Owner Author

@copilot fix failing checks
Tests / test (pull_request)
Tests / test (pull_request)Cancelled after 35m
pending checks
auto-merge
auto-mergeWaiting for status to be reported — Waiting on 1 CI check(s) (6m).
skipped checks
Docker Build and Publish / merge (pull_request)
Docker Build and Publish / merge (pull_request)Skipped yesterday
Docker Build and Publish / move-latest (pull_request)
Docker Build and Publish / move-latest (pull_request)Skipped yesterday

Copilot AI commented May 29, 2026

Copy link
Copy Markdown

@copilot fix failing checks
Tests / test (pull_request)
Tests / test (pull_request)Cancelled after 35m
...

Fixed in 4691d34 (and synced with latest branch in 14a4690). I traced the pending/cancelled state to PR workflow files diverging from main, which caused new pull_request runs to end in action_required with zero jobs. I restored .github/workflows/lint.yml and .github/workflows/tests.yml to match origin/main so CI checks can run normally on this PR.

@badMade

badMade commented May 31, 2026

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request

@badMade

badMade commented May 31, 2026

Copy link
Copy Markdown
Owner Author

@copilot fix
failing checks
Lint (ruff + ty) / ruff enforcement (blocking) (pull_request)
Lint (ruff + ty) / ruff enforcement (blocking) (pull_request)Failing after 11s

Copilot AI commented May 31, 2026

Copy link
Copy Markdown

@copilot resolve the merge conflicts in this pull request

Resolved in commit c82b0b9. I merged the latest origin/main and the latest remote PR branch updates, resolved all remaining conflicts, and pushed the updated branch.

Copilot AI commented May 31, 2026

Copy link
Copy Markdown

@copilot fix
failing checks
Lint (ruff + ty) / ruff enforcement (blocking) (pull_request)
...

The ruff enforcement failure from run 26726937797 was due to transient syntax breakage in the PR merge ref (gateway/platforms/qqbot/adapter.py and gateway/run.py), and it is already resolved on commit c82b0b9.

I revalidated on the current branch:

  • python -m py_compile gateway/platforms/qqbot/adapter.py gateway/run.py
  • ruff check . (ruff 0.15.10) ✅

The latest lint run (26726970359) is action_required with zero jobs, so it needs maintainer approval/rerun to execute checks.

@github-actions github-actions Bot merged commit 2c49828 into main May 31, 2026
1 check passed
badMade pushed a commit that referenced this pull request Jun 1, 2026
…rust roots

A prior merge widened _SANE_PATH_DIRS to include /opt/homebrew/{bin,sbin}
and /usr/local/{bin,sbin} unconditionally and made _browser_candidate_path_dirs
always inject Homebrew node prefixes. Cron / systemd / locked-down operator
configs that intentionally strip those trust roots from PATH would silently
get them injected back, defeating the restriction.

Restore #234's design:
- _SANE_PATH_DIRS only includes Termux + system dirs (/usr/{bin,sbin}, /{bin,sbin}).
- _browser_candidate_path_dirs(existing_path) takes the operator-provided PATH
  and only adds Homebrew node prefixes / /usr/local / hermes-managed Node bin
  when the operator already opted into that trust root.
- _find_agent_browser passes os.environ.get("PATH","") through to
  _merge_browser_path so the gating actually fires (previously it passed "").

Clears all 4 test_browser_homebrew_paths failures.
github-actions Bot pushed a commit that referenced this pull request Jun 2, 2026
* fix: restore session/auth helpers lost in merge conflicts

A series of merge resolutions dropped several helpers while keeping their
call sites and tests. The result was a broad cluster of NameError /
AttributeError / TypeError failures across gateway, cron, web-tools and
api-server tests.

- gateway/run.py: restore `team_id` definition in `_is_user_authorized`;
  it was deleted but two call sites still reference it.
- gateway/session_context.py: restore `get_terminal_cwd` /
  `set_terminal_cwd` / `reset_terminal_cwd` helpers (and the underlying
  `_TERMINAL_CWD` ContextVar) that run_agent.py imports.
- tools/web_tools.py: rename `_ddgs_package_importable` to
  `_ddgs_package_available` (with a backward-compat alias) so tests can
  monkeypatch the expected symbol; drop ddgs from the auto-detect
  fallback so just having the package importable doesn't silently opt
  users into a rate-limited HTML-scraping backend.
- gateway/platforms/webhook.py: reject unresolved `${VAR}` placeholder
  secrets; treating them as real HMAC secrets silently weakened auth.
- gateway/platforms/api_server.py: restore `_constant_time_equal` so
  unicode API keys compare safely instead of raising TypeError from
  `hmac.compare_digest`.

* fix(api_server): guard _constant_time_equal against None inputs

Defense in depth: the existing _check_auth caller early-returns when
self._api_key is falsy, so None can't actually reach this helper today,
but accepting Optional[str] and short-circuiting to False keeps the
helper safe for any future caller and matches the type the call site
already permits.

* fix(web_tools): delegate _ddgs_package_available to the safe helper

A bare ``import ddgs`` executes any local ``ddgs.py`` shadowing the
installed package on sys.path. The provider module already has a safer
non-importing check (metadata + find_spec, verified by
test_availability_does_not_import_shadowed_local_module). Delegate to
it so both call paths share the same protection.

* fix(approval): restore set_/reset_/get_current_run_id helpers

A prior merge removed these helpers from tools/approval.py but kept the
api_server callers that import them, breaking every /v1/runs request
with ImportError. Restore the contextvar (`_approval_run_id`) and the
three accessors so the API run path can bind a per-task run id to
pending gateway approvals again.

Clears the 7 failures under tests/gateway/test_api_server_runs.py.

* fix(api_server): restore proxy_scope authentication

A prior merge deleted gateway/proxy_scope_auth.py and stripped the
HMAC-signature check from the /v1/chat/completions handler, while
leaving the tests that exercise both paths. Restore the module and
re-wire the handler:

- Imports verify_proxy_scope_signature + signature/timestamp headers
  from gateway.proxy_scope_auth.
- Uses ``"hermes_proxy_scope" in body`` (not truthy check) so an
  explicit JSON ``null`` is rejected with 400.
- Returns 403 with "trusted gateway proxy authentication" when the
  signature is missing or invalid.

Clears the 3 TestChatCompletionsEndpoint failures.

* fix(run_agent): harden assistant-message scrub against non-str helper return

When _strip_think_blocks is mocked in tests (TestInlineThinkBlockExtraction
binds only _build_assistant_message + _extract_reasoning, leaving every
other method as a MagicMock), it returns a MagicMock instead of a string.
sanitize_context() then crashes because re.sub expects str/bytes.

Guard the scrub: if _strip_think_blocks returns a string, sanitize that;
otherwise fall back to sanitizing the original _san_content. Production
agents always return a string, so behavior there is unchanged.

Clears the 7 TestInlineThinkBlockExtraction failures.

* fix(api_server): SSRF-block private/internal image URLs

A prior merge dropped the is_safe_url() check on http(s) image URLs from
_normalize_multimodal_content, leaving only the scheme guard. Image URLs
pointing at private/internal addresses now reach the multimodal pipeline
and can exfiltrate internal-network content (test_private_image_url_rejected,
test_cloud_metadata_image_url_rejected). Re-add the check before the URL
is normalized into the image part.

* fix(tests): restore _FakeProviderMemoryManager and align memory-context test with scrub semantics

Two lost-in-merge regressions in tests/run_agent/test_run_agent.py:

1. _FakeProviderMemoryManager class was deleted but two
   TestConcurrentToolExecution tests still instantiate it, raising
   NameError. Restore the minimal double from #209.

2. test_memory_context_in_stored_content_is_preserved was renamed to
   _is_scrubbed in #478 and the assertions inverted to match the
   storage-boundary scrub the production code now performs. The pre-rename
   version kept the old "preserve" assertions, which fail against the
   correct production behaviour. Update the test to its post-#478 form.

* fix(tools_config): restore HASS_TOKEN opt-in + cross-platform MCP server fanout

Two lost-in-merge regressions in hermes_cli/tools_config.py:

- _implicit_default_off_toolsets no longer dropped homeassistant from
  default_off when HASS_TOKEN was set. That regressed Norbert's HA cron
  setup (the original PR NousResearch#14798 carve-out) because cron / cli would
  silently drop the toolset even though the operator had provisioned
  credentials. Restore the HASS_TOKEN check.

- _get_platform_tools required platform_toolsets to explicitly re-list
  every globally configured MCP server (exa, web-search-prime, etc.) to
  keep them enabled. Once a platform had any explicit builtin toolset
  list, MCP servers vanished. Restore the simpler rule: no_mcp opts out;
  otherwise enabled_mcp_servers always fan out — explicit builtin
  selection is the platform allowlist, not the MCP allowlist.

Clears 2 test_tools_config failures and the related
test_reasoning_command "exa in toolsets" assertion.

* fix(browser_tool): use url_contains_secret for navigate's secret check

The inline secret-block in browser_navigate only ran one urllib.unquote
pass, so a URL with double-percent-encoded prefixes (sk%252Dant%252D…)
slipped through and reached the browser. agent.redact.url_contains_secret
applies repeated decoding (3 passes) and also splits the URL into
component values before matching, so it catches the multi-encode tricks
that test_blocks_percent_encoded_api_key_in_url and
test_blocks_split_api_key_in_query_values exercise.

Clears 2 test_browser_secret_exfil failures.

* fix: enforce SSRF + attachment-auth gates lost in merge

Two adjacent security regressions:

- tools/browser_tool.py: pre-navigation SSRF check skipped local backends
  (`not _is_local_backend()` short-circuited the guard) even though the
  surrounding comment explicitly states local backends must enforce it
  too — browser_snapshot can return local-file / internal-service
  responses in reduced-tool configurations. Drop the local-backend skip
  so the guard fires unless the operator opts in via
  ``browser.allow_private_urls``.

- gateway/platforms/qqbot/adapter.py: restore the attachment pre-auth
  gate from #349. _handle_c2c/_handle_group/_handle_guild/_handle_dm
  now check `_is_source_authorized_for_attachment_processing` before
  calling `_process_attachments`, and forward a text-only event when
  the sender isn't allowlisted. This prevents an unauthorized sender
  from forcing the bot to fetch attacker-controlled attachment URLs
  (SSRF amplification, large-file DoS, redirect attacks). Failure-closed
  when gateway_runner isn't attached yet, with a throttled warning so
  startup races don't spam the log.

Clears 2 test_browser_ssrf_local failures and
test_unauthorized_c2c_skips_attachment_processing.

* fix(kanban): scope worker child reaping to known PIDs only

dispatch_once() was calling waitpid(-1, WNOHANG) on every tick, which
reaps any zombie child of the gateway process — including non-kanban
subprocesses (npm install, agent-browser, etc.) whose callers rely on
their own Popen.wait()/subprocess.run() exit status. That broke
unrelated tools whenever the kanban dispatcher ran in the same process.

Restore the scoped reaper from #393: track each kanban worker PID in
_known_worker_child_pids when it's persisted via _set_worker_pid, and
in dispatch_once only waitpid those specific PIDs. Windows is still a
no-op (no zombies / no WNOHANG).

Clears test_source_gates_waitpid_loop.

* fix(api_server): propagate SSE batch flush failures to main streaming loop

When the batched-delta background task ("_batch_flush_after") hit a
ConnectionResetError on response.write(), the exception was swallowed
in the detached task and the main loop kept waiting on stream_q for
items that would never arrive — the streaming endpoint hung until the
client timed out and the agent was never interrupted.

Restore #398's fix: catch the flush exception in the background task,
stash it in _batch_error, and push a sentinel into stream_q so the main
loop re-raises it on dequeue. Both the live loop and the drain path
honour the sentinel.

Clears test_stream_batched_delta_disconnect_interrupts_agent.

* fix(browser_tool): honor restricted PATH for Homebrew/user-writable trust roots

A prior merge widened _SANE_PATH_DIRS to include /opt/homebrew/{bin,sbin}
and /usr/local/{bin,sbin} unconditionally and made _browser_candidate_path_dirs
always inject Homebrew node prefixes. Cron / systemd / locked-down operator
configs that intentionally strip those trust roots from PATH would silently
get them injected back, defeating the restriction.

Restore #234's design:
- _SANE_PATH_DIRS only includes Termux + system dirs (/usr/{bin,sbin}, /{bin,sbin}).
- _browser_candidate_path_dirs(existing_path) takes the operator-provided PATH
  and only adds Homebrew node prefixes / /usr/local / hermes-managed Node bin
  when the operator already opted into that trust root.
- _find_agent_browser passes os.environ.get("PATH","") through to
  _merge_browser_path so the gating actually fires (previously it passed "").

Clears all 4 test_browser_homebrew_paths failures.

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants