Skip to content

fix(agents-md): security hardening dropped from #5087 merge - #5096

Merged
senamakel merged 3 commits into
tinyhumansai:mainfrom
senamakel:fix/agents-md-security-followup
Jul 21, 2026
Merged

senamakel merged 3 commits into
tinyhumansai:mainfrom
senamakel:fix/agents-md-security-followup

Conversation

@senamakel

@senamakel senamakel commented Jul 21, 2026 •

Copy link
Copy Markdown
Member

Summary

Problem

  • A project-controlled AGENTS.md could be a symlink pointing at a secret outside the root; the loader would read it and the content would flow into the system prompt (and on to the inference provider).
  • The full-body system-prompt debug log could print raw AGENTS.md content verbatim when PROFILE/MEMORY were omitted.
  • Both fixes had approved/resolved review threads on feat(prompts): load AGENTS.md project instructions into the system prompt #5087 but are absent from main because the merge happened before the head ref propagated.

Solution

  • Loader refuses symlinked AGENTS.md and validates the canonical path stays under its root (validate_path_within_root) before reading; Unix regression test added.
  • AGENTS.md presence added to the debug-log redaction trigger so prompt bodies containing it are never logged raw.

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy — symlink-rejection and escape-path regression tests; 23 agents_md tests pass
  • Diff coverage ≥ 80% — changed lines covered by the new agents_md tests
  • N/A: Coverage matrix updated — no feature rows added/removed/renamed
  • N/A: All affected feature IDs from the matrix are listed under ## Related — no matrix rows affected
  • No new external network dependencies introduced (local file handling only)
  • N/A: Manual smoke checklist — does not touch release-cut surfaces
  • N/A: Linked issue closed via Closes #NNN — follow-up to PR feat(prompts): load AGENTS.md project instructions into the system prompt #5087 review threads, no standalone issue

Impact

  • Closes a local file-disclosure vector (symlinked AGENTS.md → prompt → provider) and a PII/content leak into debug logs. No behaviour change for regular non-symlinked AGENTS.md files.

Related


AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/agents-md-security-followup
  • Commit SHA: 6b97e64

Validation Run

  • N/A pnpm --filter openhuman-app format:check — no frontend changes
  • N/A pnpm typecheck — no TypeScript changes
  • Focused tests: cargo test --lib agents_md (23 passed)
  • Rust fmt/check (if changed): GGML_NATIVE=OFF cargo check clean, cargo fmt --check clean
  • N/A Tauri fmt/check (if changed): app/src-tauri untouched

Validation Blocked

  • command: N/A
  • error: N/A
  • impact: N/A

Behavior Changes

  • Intended behavior change: symlinked or root-escaping AGENTS.md files are refused; prompts containing AGENTS.md are redacted in debug logs
  • User-visible effect: none for normal AGENTS.md usage

Parity Contract

  • Legacy behavior preserved: regular file loading, layering, capping, and gating unchanged
  • Guard/fallback/dispatch parity checks: loader is the single shared path for main + subagent layers, so both are covered

Duplicate / Superseded PR Handling

Summary by CodeRabbit

  • Security Enhancements

    • Improved protection of sensitive prompt content in debug logs, including instructions from AGENTS.md.
    • Prevented loading AGENTS.md files that are symlinks or located outside the permitted directory.
    • Added safeguards for invalid or unreadable files while preserving size limits and truncation behavior.
  • Bug Fixes

    • Sensitive prompt details are now consistently redacted when applicable.

The project-layer AGENTS.md lives in the user/project-controlled action dir, so
a checkout could symlink it to a secret outside the root (e.g. ~/.ssh/id_rsa) or
a blocking device and have its bytes read into the system prompt and sent to the
inference provider. Refuse symlinks, require a regular file, and require the
canonical path to stay under its root (validate_path_within_root) before
reading. Adds a unix symlink-rejection test.
The full-body prompt debug log was taken whenever PROFILE.md + MEMORY.md were
both omitted (common for narrow specialists), but AGENTS.md instruction layers
can land in the prompt independently and would then be logged verbatim. Add
AGENTS.md presence to the redaction trigger so its user/project content is
hashed, not printed.
@senamakel
senamakel requested a review from a team July 21, 2026 13:48
@coderabbitai

coderabbitai Bot commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

An error occurred during the review process. Please try again later.

📝 Walkthrough

Walkthrough

The changes harden AGENTS.md loading against symlinks and paths outside the action root, add Unix coverage for external symlinks, and redact system-prompt debug logs when AGENTS.md instructions are present.

Changes

AGENTS.md security and prompt redaction

Layer / File(s) Summary
Secure AGENTS.md loading
src/openhuman/agent/prompts/agents_md.rs
load_agents_md rejects symlinked and non-regular files, validates canonical paths within the root, and adds a Unix symlink rejection test.
Redact prompts containing AGENTS.md
src/openhuman/agent/harness/session/turn/core.rs
Agent::turn uses the presence of the AGENTS.md instruction layer as an additional condition for hashed system-prompt logging.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: agent, bug

Suggested reviewers: sanil-23

Poem

I hop through guarded paths with care,
No sneaky links may enter there.
AGENTS’ words stay safely veiled,
While prompt logs show hashes mailed.
Thump, thump—secure code everywhere!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main theme of restoring AGENTS.md security hardening that was lost in the #5087 merge.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added agent Built-in agents, prompts, orchestration, and agent runtime in src/openhuman/agent/. bug labels Jul 21, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/openhuman/agent/prompts/agents_md.rs`:
- Around line 80-128: Eliminate the check-to-open race in the AGENTS.md loading
flow by changing the path-based validation around the loader function to open
the file relative to the trusted root directory handle with platform-appropriate
no-follow semantics, then validate the opened handle’s type before reading it.
Preserve rejection of symlinked or non-regular files and add a regression test
through an injectable filesystem/open seam that replaces the checked file before
opening, confirming the symlink is rejected.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6f18ee40-61c3-4138-9227-41cda38ea05c

📥 Commits

Reviewing files that changed from the base of the PR and between c7f67b7 and 6b97e64.

📒 Files selected for processing (2)
  • src/openhuman/agent/harness/session/turn/core.rs
  • src/openhuman/agent/prompts/agents_md.rs

The prior symlink_metadata check + later File::open by path left a TOCTOU
window: a writable action dir could swap the checked regular AGENTS.md for a
symlink before open, and File::open would follow it to a target outside the
root. Open with O_NOFOLLOW (Unix) so the kernel atomically refuses a
final-component symlink at open time (no window), plus O_NONBLOCK so a
substituted FIFO/device can't block; then fstat the opened handle and require a
regular file. validate_path_within_root stays as parent-component defence in
depth. Adds a FIFO-rejection regression test; the symlink test now exercises the
race-free path.
@senamakel
senamakel merged commit 4df2e24 into tinyhumansai:main Jul 21, 2026
19 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Team Openhuman Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Built-in agents, prompts, orchestration, and agent runtime in src/openhuman/agent/. bug

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant