test(kanban): cover kanban_comment author hardening + cross-task policy

- Renames test_comment_custom_author -> test_comment_ignores_caller_supplied_author
  and inverts its assertion: an args['author'] override is silently
  ignored; the author always comes from HERMES_PROFILE.
- Adds test_comment_schema_omits_author_override to assert the
  'author' property is gone from KANBAN_COMMENT_SCHEMA so the
  forgery surface stays closed if someone re-adds the schema field
  by accident.
- Adds test_worker_can_comment_on_foreign_task to pin the #19713
  policy decision: cross-task commenting must remain unrestricted.
  Without this guard, a future change accidentally adding
  _enforce_worker_task_ownership to _handle_comment would close the
  documented handoff channel between tasks.
This commit is contained in:
kshitijk4poor 2026-05-09 14:46:27 +05:30 committed by kshitij
parent 9bbad3cc10
commit e3ebaa19ba

View file

@ -296,21 +296,40 @@ def test_comment_rejects_empty_body(worker_env):
assert json.loads(out).get("error")
def test_comment_custom_author(worker_env):
def test_comment_ignores_caller_supplied_author(worker_env):
"""``args["author"]`` is no longer honored — the author is always
derived from ``HERMES_PROFILE`` so a worker can't forge a comment
under an authoritative-looking name like ``hermes-system`` and
poison the next worker's prompt context. Cross-task commenting
itself remains unrestricted (see #19713); only the author override
is removed.
"""
from tools import kanban_tools as kt
out = kt._handle_comment({
"task_id": worker_env, "body": "hi", "author": "custom-bot",
"task_id": worker_env, "body": "hi", "author": "hermes-system",
})
assert json.loads(out)["ok"]
from hermes_cli import kanban_db as kb
conn = kb.connect()
try:
comments = kb.list_comments(conn, worker_env)
assert comments[0].author == "custom-bot"
# Author comes from HERMES_PROFILE in the fixture, not the
# caller-supplied "hermes-system" override.
assert comments[0].author == "test-worker"
finally:
conn.close()
def test_comment_schema_omits_author_override():
"""The ``author`` property must not appear on KANBAN_COMMENT_SCHEMA;
exposing it to the LLM would re-introduce the forgery surface this
handler is hardened against.
"""
from tools.kanban_tools import KANBAN_COMMENT_SCHEMA
props = KANBAN_COMMENT_SCHEMA["parameters"]["properties"]
assert "author" not in props
def test_create_happy_path(worker_env):
from tools import kanban_tools as kt
out = kt._handle_create({
@ -657,6 +676,42 @@ def test_worker_heartbeat_rejects_foreign_task_id(worker_env):
assert "refusing to mutate" in d.get("error", "")
def test_worker_can_comment_on_foreign_task(worker_env):
"""Cross-task commenting must remain unrestricted (#19713 policy).
The author-forgery hardening removed args['author'] but deliberately
did NOT add an ownership gate to kanban_comment comments are the
documented handoff channel between tasks. This test pins that policy
so a future change accidentally adding ``_enforce_worker_task_ownership``
to ``_handle_comment`` would fail CI immediately.
"""
from hermes_cli import kanban_db as kb
conn = kb.connect()
try:
other = kb.create_task(conn, title="sibling")
finally:
conn.close()
from tools import kanban_tools as kt
out = kt._handle_comment({
"task_id": other,
"body": "handoff: see prior findings before starting",
})
d = json.loads(out)
assert d.get("ok") is True, f"cross-task comment must succeed: {d}"
# The comment lands on the foreign task, attributed to the worker's
# HERMES_PROFILE — never to a caller-controlled string.
conn = kb.connect()
try:
comments = kb.list_comments(conn, other)
assert len(comments) == 1
assert comments[0].author == "test-worker"
assert comments[0].body.startswith("handoff:")
finally:
conn.close()
def test_worker_complete_own_task_still_works(worker_env):
"""The ownership check doesn't break the normal own-task happy path."""
from tools import kanban_tools as kt