Skip to content

feat(kanban): attachment toolset + CLI with SSRF-guarded URL fetch (#36019 salvage) - #65698

Merged
teknium1 merged 5 commits into
mainfrom
salvage/36019-kanban-attachments
Jul 16, 2026
Merged

feat(kanban): attachment toolset + CLI with SSRF-guarded URL fetch (#36019 salvage)#65698
teknium1 merged 5 commits into
mainfrom
salvage/36019-kanban-attachments

Conversation

@teknium1

@teknium1 teknium1 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Kanban attachments are now reachable from agent tools and the CLI, matching the dashboard surface added in #35338: kanban_attach (base64), kanban_attach_url (server-side fetch), kanban_attachments (list) + CLI verbs attach/attachments/attach-rm, all built on a shared store_attachment_bytes() in kanban_db.py.

Salvage of #36019 by @otsune with both review blockers fixed on top (authorship preserved):

  1. SSRF guard: kanban_attach_url's fetch now routes through tools/url_safety.py's canonical guard — loopback, private ranges, cloud-metadata endpoints, and redirects-to-private are rejected (tests for 127.0.0.1, 169.254.169.254, 10.0.0.1, public→loopback redirect, plus mocked happy path).
  2. Cap unification: dropped the PR's private _MAX_ATTACHMENT_BYTES in favor of main's centralized KANBAN_ATTACHMENT_MAX_BYTES; kept main's plugin_api.py helpers and adapted the tools to them.

Toolset footprint follows the accepted kanban pattern: all three tools gated by check_fn=_check_kanban_mode (zero schema footprint outside dispatcher-spawned kanban tasks), write tools enforce _enforce_worker_task_ownership. The PR's kanban-worker SKILL.md hunk was re-targeted to KANBAN_GUIDANCE (main folded that skill in via #50473), staying under the guidance size bound.

Changes

11 files, +1172/−41 — kanban_db shared helper, tools, CLI verbs, dashboard refactor onto shared path, guidance, tests, AUTHOR_MAP.

Validation

  • 5 kanban test files: 327 passed, 0 failed (scripts/run_tests.sh)
  • SSRF rejection matrix covered per above; ruff clean; working tree clean

Infographic

infographic

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cron Cron scheduler and job management comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets labels Jul 16, 2026
tonydwb
tonydwb reviewed Jul 16, 2026
View reviewed changes

@tonydwb tonydwb 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.

Code Review Summary

Verdict: Comment

Overview

  • Kanban attachment toolset + CLI with SSRF-guarded URL fetch
  • 1172 additions, 41 deletions

Assessment

  • SSRF protection on URL fetch is good security practice
  • CLI tool for attachments adds useful functionality
  • Multiple print() statements to stderr for error reporting are acceptable for CLI tools

Suggestion

  • Ensure the SSRF guard validates both the host and path components of URLs to prevent http://169.254.169.254/ style attacks

Reviewed by Hermes Agent

otsune and others added 5 commits July 16, 2026 07:28
@otsune @claude @teknium1
feat(kanban): attachment toolset + CLI to match the dashboard surface
9ec76fd 
The kanban board has had full attachment storage and a dashboard HTTP
API (upload/list/download/delete) since #35338, but there was no agent
toolset tool and no `hermes kanban` CLI verb for attachments. Agents and
scripts that don't go through the dashboard server (or can't touch the DB
directly) had no way to create or read real attachments — only links in
comments.

Close that gap by mirroring the existing comment surface:

- `kanban_db.store_attachment_bytes()` — one shared write path (validate
  name, enforce the 25 MB cap, write the blob under the per-task dir with
  collision-free naming, insert the metadata row, clean up an orphan blob
  if the insert fails). `_MAX_ATTACHMENT_BYTES`, `_safe_attachment_name`,
  and a new `_collision_free_path` move here so the dashboard, the tool,
  and the CLI all share one implementation and can't drift.
- Tools (`tools/kanban_tools.py`): `kanban_attach` (inline base64),
  `kanban_attach_url` (server-side http/https fetch with the same cap),
  `kanban_attachments` (list). Write tools respect worker task-ownership;
  list is read-only. Registered in the `kanban` toolset.
- CLI (`hermes_cli/kanban.py`): `attach <id> <path>`, `attachments <id>`,
  `attach-rm <attachment_id>`.
- Dashboard `upload_task_attachment` now imports the shared helpers and
  uses `_collision_free_path` — behavior identical (still streams to disk
  with the cap, still 413 on overflow).
- Docs (AGENTS.md, kanban-worker skill) and toolset membership updated.

Tests: tool round-trip + oversize + bad base64 + ownership; attach_url
against a local HTTP fixture incl. oversize-mid-stream and non-http
scheme rejection; CLI attach/attachments/attach-rm; shared-helper unit
tests; dashboard parity preserved.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@teknium1
refactor(kanban): unify attachment size cap on KANBAN_ATTACHMENT_MAX_…
ac264e8 
…BYTES

The salvaged attachment-toolset commit predated main centralizing the
25 MB cap as kanban_db.KANBAN_ATTACHMENT_MAX_BYTES and re-introduced a
private _MAX_ATTACHMENT_BYTES alias. Drop the duplicate: kanban_db's
store_attachment_bytes(), the dashboard upload endpoint, and the
kanban_attach_url tool all reference the one shared constant now, and
the tests monkeypatch that same name.
@teknium1
fix(kanban): guard kanban_attach_url against SSRF via tools.url_safety
2d4f327 
_download_url_with_cap called urlopen() after only a scheme check, so a
model-controlled URL could reach loopback services, RFC1918/CGNAT hosts,
or cloud metadata endpoints (169.254.169.254), and a public host could
302 to any of those unvalidated.

Route the fetch through the repo's canonical SSRF guard instead:
validate every hop with tools.url_safety.is_safe_url() and follow
redirects manually (httpx, follow_redirects=False, 5-hop limit) so each
Location target is re-checked before it is fetched — the same pattern
as tools/skills_hub._guarded_http_get. The streaming size cap is
unchanged. Local-fixture tests opt in via HERMES_ALLOW_PRIVATE_URLS
(the guard's documented escape hatch); new tests pin rejection of
loopback, cloud-metadata, and private-range URLs, a mocked
public→loopback redirect, and a mocked public happy path.
@teknium1
chore(release): map otsune's noreply email in AUTHOR_MAP (PR #36019 s…
54bf5fc 
…alvage)
@teknium1
docs(kanban): port attachment guidance into KANBAN_GUIDANCE
8e3372e 
PR #36019 documented the attachment tools in the kanban-worker skill,
but main removed that skill in #50473 and folded its content into the
KANBAN_GUIDANCE prompt block. Land the same guidance there instead so
every dispatcher-spawned worker sees it.
@teknium1
teknium1 force-pushed the salvage/36019-kanban-attachments branch from 82b62dd to 8e3372e Compare July 16, 2026 14:28
@teknium1
teknium1 merged commit b5bd0ef into main Jul 16, 2026
31 checks passed
@teknium1
teknium1 deleted the salvage/36019-kanban-attachments branch July 16, 2026 14:33
@teknium1 teknium1 mentioned this pull request Jul 16, 2026
Closed
This was referenced Jul 20, 2026
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@tonydwb tonydwb tonydwb left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cron Cron scheduler and job management comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@teknium1 @tonydwb @alt-glitch @otsune