Skip to content

fix: restore critical security guardrails #675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
badMade wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: restore critical security guardrails #675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7486abd
fix: restore critical security guardrails
badMade Jun 1, 2026
ff0a791
fix: address failing CI checks for security guardrail PR
Copilot Jun 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 72 additions & 19 deletions .github/workflows/nix-lockfile-fix.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ on:
types: [edited]

permissions:
contents: write
contents: read
pull-requests: write

concurrency:
Expand Down Expand Up @@ -48,6 +48,9 @@ jobs:
concurrency:
group: auto-fix-main
cancel-in-progress: true
permissions:
contents: write
pull-requests: write
steps:
- name: Generate GitHub App token
id: app-token
Expand Down Expand Up @@ -122,8 +125,9 @@ jobs:
exit 1

# ── PR fix (manual / checkbox) ─────────────────────────────────────
# Existing behavior: run on manual dispatch OR when a task-list
# checkbox in the sticky lockfile-check comment flips from [ ] to [x].
# Runs untrusted PR content as data only: the base-repository checkout owns
# the action and Nix code that receive secrets; the PR checkout never gets
# persisted credentials or Cachix credentials.
fix:
if: |
github.event_name == 'workflow_dispatch' ||
Expand All @@ -133,14 +137,15 @@ jobs:
&& !contains(github.event.changes.body.from, '[x] **Apply lockfile fix**'))
runs-on: ubuntu-latest
timeout-minutes: 25
permissions:
contents: write
pull-requests: write
steps:
- name: Authorize & resolve PR
id: resolve
uses: actions/github-script@d746ffe35508b1917358783b479e04febd2b8f71 # v9
with:
script: |
// 1. Verify the actor has write access — applies to both checkbox
// clicks and manual dispatch.
const { data: perm } =
await github.rest.repos.getCollaboratorPermissionLevel({
owner: context.repo.owner,
Expand All @@ -154,19 +159,20 @@ jobs:
return;
}

// 2. Resolve which ref to check out.
let prNumber = '';
if (context.eventName === 'issue_comment') {
prNumber = String(context.payload.issue.number);
} else if (context.eventName === 'workflow_dispatch') {
prNumber = context.payload.inputs.pr_number || '';
}

core.setOutput('trusted_ref', context.payload.repository.default_branch);
if (!prNumber) {
core.setOutput('ref', context.ref.replace(/^refs\/heads\//, ''));
core.setOutput('repo', context.repo.repo);
core.setOutput('owner', context.repo.owner);
core.setOutput('pr', '');
core.setOutput('head_sha', '');
return;
}

Expand All @@ -179,10 +185,8 @@ jobs:
core.setOutput('repo', pr.head.repo.name);
core.setOutput('owner', pr.head.repo.owner.login);
core.setOutput('pr', String(pr.number));
core.setOutput('head_sha', pr.head.sha);

# Wipe the sticky lockfile-check comment to a "running" state as soon
# as the job is authorized, so the user sees their click was picked up
# before the ~minute of nix build work.
- name: Mark sticky as running
if: steps.resolve.outputs.pr != ''
uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3
Expand All @@ -194,34 +198,83 @@ jobs:

Triggered by @${{ github.actor }} — [workflow run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}).

- uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
- name: Checkout trusted workflow code
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
with:
repository: ${{ github.repository }}
ref: ${{ steps.resolve.outputs.trusted_ref }}
path: trusted
persist-credentials: false

- name: Checkout target tree without credentials
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
with:
repository: ${{ steps.resolve.outputs.owner }}/${{ steps.resolve.outputs.repo }}
ref: ${{ steps.resolve.outputs.ref }}
token: ${{ secrets.GITHUB_TOKEN }}
ref: ${{ steps.resolve.outputs.head_sha || steps.resolve.outputs.ref }}
path: target
fetch-depth: 0
persist-credentials: false
token: ''

- uses: ./.github/actions/nix-setup
- name: Overlay package manifests onto trusted checkout
shell: bash
run: |
set -euo pipefail
for f in \
ui-tui/package.json \
ui-tui/package-lock.json \
web/package.json \
web/package-lock.json
do
cp "target/$f" "trusted/$f"
done

- uses: ./trusted/.github/actions/nix-setup
with:
cachix-auth-token: ${{ secrets.CACHIX_AUTH_TOKEN }}

- name: Apply lockfile hashes
- name: Apply lockfile hashes using trusted Nix code
id: apply
run: nix run .#fix-lockfiles
working-directory: trusted
run: nix run .#fix-lockfiles -- --apply

- name: Copy generated lockfile hashes
shell: bash
run: |
set -euo pipefail
cp trusted/nix/tui.nix target/nix/tui.nix
cp trusted/nix/web.nix target/nix/web.nix
if git -C target diff --quiet -- nix/tui.nix nix/web.nix; then
echo "changed=false" >> "$GITHUB_OUTPUT"
else
echo "changed=true" >> "$GITHUB_OUTPUT"
fi
id: copy

- name: Commit & push
if: steps.apply.outputs.changed == 'true'
if: steps.copy.outputs.changed == 'true'
working-directory: target
shell: bash
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
TARGET_OWNER: ${{ steps.resolve.outputs.owner }}
TARGET_REPO: ${{ steps.resolve.outputs.repo }}
TARGET_REF: ${{ steps.resolve.outputs.ref }}
run: |
set -euo pipefail
unexpected="$(git diff --name-only | grep -Ev '^nix/(tui|web)\.nix$' || true)"
if [ -n "$unexpected" ]; then
echo "::error::Unexpected modified files: $unexpected"
exit 1
fi
git config user.name 'github-actions[bot]'
git config user.email '41898282+github-actions[bot]@users.noreply.github.com'
git add nix/tui.nix nix/web.nix
git commit -m "fix(nix): refresh npm lockfile hashes"
git push
git push "https://x-access-token:${GH_TOKEN}@github.com/${TARGET_OWNER}/${TARGET_REPO}.git" "HEAD:refs/heads/${TARGET_REF}"

- name: Update sticky (applied)
if: steps.apply.outputs.changed == 'true' && steps.resolve.outputs.pr != ''
if: steps.copy.outputs.changed == 'true' && steps.resolve.outputs.pr != ''
uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3
with:
header: nix-lockfile-check
Expand All @@ -232,7 +285,7 @@ jobs:
Pushed a commit refreshing the npm lockfile hashes — [workflow run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}).

- name: Update sticky (already current)
if: steps.apply.outputs.changed == 'false' && steps.resolve.outputs.pr != ''
if: steps.copy.outputs.changed == 'false' && steps.resolve.outputs.pr != ''
uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3
with:
header: nix-lockfile-check
Expand Down
48 changes: 45 additions & 3 deletions acp_adapter/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@
from acp_adapter.permissions import make_approval_callback
from acp_adapter.session import SessionManager, SessionState, _expand_acp_enabled_toolsets
from acp_adapter.tools import build_tool_complete, build_tool_start
from agent.file_safety import get_read_block_error
from agent.redact import redact_sensitive_text

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -208,7 +210,35 @@ def _format_resource_text(
return f"{header}\nURI: {uri}\n\n{body}"


def _resource_link_to_parts(block: ResourceContentBlock) -> list[dict[str, Any]]:
def _resolve_acp_resource_path(path: Path, cwd: Path | None) -> tuple[Path | None, str | None]:
"""Resolve an ACP file resource under the session cwd and read policy."""
if cwd is None:
return None, "ACP file resources require an active session cwd."

try:
root = cwd.expanduser().resolve()
Comment on lines +213 to +219
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Jun 1, 2026

Choose a reason for hiding this comment

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

high

The cwd parameter can be passed as a string (e.g., from the session manager or ACP client payloads). Calling cwd.expanduser() directly on a string will raise an AttributeError, crashing the resource resolution. Wrapping cwd in Path(cwd) ensures robust handling of both Path and str inputs.

Suggested change
def _resolve_acp_resource_path(path: Path, cwd: Path | None) -> tuple[Path | None, str | None]:
"""Resolve an ACP file resource under the session cwd and read policy."""
if cwd is None:
return None, "ACP file resources require an active session cwd."
try:
root = cwd.expanduser().resolve()
def _resolve_acp_resource_path(path: Path, cwd: Path | str | None) -> tuple[Path | None, str | None]:
"""Resolve an ACP file resource under the session cwd and read policy."""
if cwd is None:
return None, "ACP file resources require an active session cwd."
try:
root = Path(cwd).expanduser().resolve()

candidate = path.expanduser()
if not candidate.is_absolute():
candidate = root / candidate
resolved = candidate.resolve()
resolved.relative_to(root)
except (OSError, ValueError) as exc:
return None, f"Access denied: file resource is outside the ACP session directory ({exc})."

block_error = get_read_block_error(str(resolved))
if block_error:
return None, block_error

try:
if not resolved.is_file():
return None, "Access denied: ACP file resource is not a regular file."
except OSError as exc:
return None, f"Access denied: could not inspect ACP file resource ({exc})."

return resolved, None


def _resource_link_to_parts(block: ResourceContentBlock, *, cwd: Path | None = None) -> list[dict[str, Any]]:
"""Convert an ACP resource_link block to OpenAI content parts.

Returns a list of {"type": "text", ...} and/or {"type": "image_url", ...}
Expand Down Expand Up @@ -236,6 +266,15 @@ def _resource_link_to_parts(block: ResourceContentBlock) -> list[dict[str, Any]]
),
}]

path, path_error = _resolve_acp_resource_path(path, cwd)
if path_error:
return [{
"type": "text",
"text": _format_resource_text(uri=uri, name=name, title=title, body=f"[{path_error}]"),
}]
if path is None:
return []

# Image files: emit a short text header + image_url data URL so vision
# models can see the attachment instead of a "binary omitted" note.
image_mime = mime_type if _is_image_resource(mime_type) else _guess_image_mime_from_path(path)
Expand Down Expand Up @@ -288,6 +327,7 @@ def _resource_link_to_parts(block: ResourceContentBlock) -> list[dict[str, Any]]
),
}]
note = None
text = redact_sensitive_text(text, code_file=True)
if size > _MAX_ACP_RESOURCE_BYTES:
note = f"truncated to {_MAX_ACP_RESOURCE_BYTES} of {size} bytes"
return [{
Expand Down Expand Up @@ -399,6 +439,8 @@ def _content_blocks_to_openai_user_content(
| ResourceContentBlock
| EmbeddedResourceContentBlock
],
*,
cwd: Path | None = None,
) -> str | list[dict[str, Any]]:
"""Convert ACP prompt blocks into a Hermes/OpenAI-compatible user content payload."""
parts: list[dict[str, Any]] = []
Expand All @@ -416,7 +458,7 @@ def _content_blocks_to_openai_user_content(
parts.append(image_part)
continue
if isinstance(block, ResourceContentBlock):
resource_parts = _resource_link_to_parts(block)
resource_parts = _resource_link_to_parts(block, cwd=cwd)
for part in resource_parts:
parts.append(part)
if part.get("type") == "text":
Expand Down Expand Up @@ -1087,7 +1129,7 @@ async def prompt(
return PromptResponse(stop_reason="refusal")

user_text = _extract_text(prompt).strip()
user_content = _content_blocks_to_openai_user_content(prompt)
user_content = _content_blocks_to_openai_user_content(prompt, cwd=state.cwd)
text_only_prompt = all(isinstance(block, TextContentBlock) for block in prompt)
has_content = bool(user_text) or (
isinstance(user_content, list) and bool(user_content)
Expand Down
51 changes: 50 additions & 1 deletion gateway/platforms/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,9 +470,55 @@ def is_host_excluded_by_no_proxy(hostname: str, no_proxy_value: str | None = Non

from gateway.config import Platform, PlatformConfig
from gateway.session import SessionSource, build_session_key
from hermes_constants import get_hermes_dir
from hermes_constants import get_hermes_dir, get_hermes_home


def _trusted_gateway_media_roots() -> tuple[Path, ...]:
"""Return Hermes-managed directories allowed for model-emitted attachments."""
home = get_hermes_home()
roots = [
get_image_cache_dir(),
get_audio_cache_dir(),
get_video_cache_dir(),
get_document_cache_dir(),
get_hermes_dir("cache/screenshots", "browser_screenshots"),
get_hermes_dir("cache/vision", "temp_vision_images"),
home / "cache" / "images",
home / "cache" / "audio",
home / "cache" / "videos",
home / "cache" / "documents",
]
resolved: list[Path] = []
for root in roots:
try:
resolved.append(Path(root).expanduser().resolve())
except OSError:
continue
return tuple(dict.fromkeys(resolved))
Comment on lines +476 to +497
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Jun 1, 2026

Choose a reason for hiding this comment

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

medium

The _trusted_gateway_media_roots function is called on every invocation of _is_trusted_gateway_media_path during path filtering. This repeatedly executes multiple mkdir(parents=True, exist_ok=True) and resolve() system calls, which is highly inefficient. Caching the resolved roots at the module level avoids this redundant I/O overhead.

_TRUSTED_MEDIA_ROOTS_CACHE: tuple[Path, ...] | None = None


def _trusted_gateway_media_roots() -> tuple[Path, ...]:
    """Return Hermes-managed directories allowed for model-emitted attachments."""
    global _TRUSTED_MEDIA_ROOTS_CACHE
    if _TRUSTED_MEDIA_ROOTS_CACHE is not None:
        return _TRUSTED_MEDIA_ROOTS_CACHE
    home = get_hermes_home()
    roots = [
        get_image_cache_dir(),
        get_audio_cache_dir(),
        get_video_cache_dir(),
        get_document_cache_dir(),
        get_hermes_dir("cache/screenshots", "browser_screenshots"),
        get_hermes_dir("cache/vision", "temp_vision_images"),
        home / "cache" / "images",
        home / "cache" / "audio",
        home / "cache" / "videos",
        home / "cache" / "documents",
    ]
    resolved: list[Path] = []
    for root in roots:
        try:
            resolved.append(Path(root).expanduser().resolve())
        except OSError:
            continue
    _TRUSTED_MEDIA_ROOTS_CACHE = tuple(dict.fromkeys(resolved))
    return _TRUSTED_MEDIA_ROOTS_CACHE



def _is_trusted_gateway_media_path(path: str) -> bool:
"""Return True when *path* is a regular file inside Hermes media caches."""
try:
candidate = Path(path).expanduser().resolve()
if not candidate.is_file():
return False
return any(candidate.is_relative_to(root) for root in _trusted_gateway_media_roots())
except OSError:
return False
Comment on lines +500 to +508
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Jun 1, 2026

Choose a reason for hiding this comment

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

security-high high

If path is not a string or Path-like object, or if it contains null bytes (e.g., from malicious model output), Path(path) or resolve() can raise a TypeError or ValueError. Since the try block only catches OSError, these unhandled exceptions will crash the gateway process. Catching (OSError, TypeError, ValueError) ensures robust and safe execution.

Suggested change
def _is_trusted_gateway_media_path(path: str) -> bool:
"""Return True when *path* is a regular file inside Hermes media caches."""
try:
candidate = Path(path).expanduser().resolve()
if not candidate.is_file():
return False
return any(candidate.is_relative_to(root) for root in _trusted_gateway_media_roots())
except OSError:
return False
def _is_trusted_gateway_media_path(path: str) -> bool:
"""Return True when *path* is a regular file inside Hermes media caches."""
try:
candidate = Path(path).expanduser().resolve()
if not candidate.is_file():
return False
return any(candidate.is_relative_to(root) for root in _trusted_gateway_media_roots())
except (OSError, TypeError, ValueError):
return False



def _filter_trusted_gateway_media_paths(paths):
"""Drop model-emitted local paths outside Hermes-managed media caches."""
trusted = []
for item in paths:
path = item[0] if isinstance(item, tuple) else item
if _is_trusted_gateway_media_path(path):
trusted.append(item)
else:
logger.warning("Blocked untrusted model-emitted media path: %s", path)
return trusted

GATEWAY_SECRET_CAPTURE_UNSUPPORTED_MESSAGE = (
"Secure secret entry is not supported over messaging. "
"Load this skill in the local CLI to be prompted, or add the key to ~/.hermes/.env manually."
Expand Down Expand Up @@ -2982,6 +3028,8 @@ async def _stop_typing_task() -> None:
# Extract MEDIA:<path> tags (from TTS tool) before other processing
media_files, response = self.extract_media(response)

media_files = _filter_trusted_gateway_media_paths(media_files)

# Extract image URLs and send them as native platform attachments
images, text_content = self.extract_images(response)
# Strip any remaining internal directives from message body (fixes #1561)
Expand All @@ -2994,6 +3042,7 @@ async def _stop_typing_task() -> None:
# Auto-detect bare local file paths for native media delivery
# (helps small models that don't use MEDIA: syntax)
local_files, text_content = self.extract_local_files(text_content)
local_files = _filter_trusted_gateway_media_paths(local_files)
if local_files:
logger.info("[%s] extract_local_files found %d file(s) in response", self.name, len(local_files))

Expand Down
Loading
Loading