Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions changelog.d/tsk-fjy3o7-mention-revocation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
### Fixed: identity comparison and revocation check normalised

- Fixed `taosmd/registry_auth.py:authorize_sender` to normalise handle identity
using :func:`taosmd.service._normalise_handle` in both the ``sub != claimed_from``
comparison (line 83) and the revocation check (line 92), so ``@``-prefixed and
bare handles compare equal case-insensitively across both code paths.

### Added: taosmd/mentions.py

- Created `taosmd/mentions.py` with `MentionStore` class for recording and
querying ``@``-handle mentions on the A2A bus, enabling the
``/a2a/mentions`` feed and thread-scoped visibility anti-bypass rule from
taOSmd ``#211``. The store normalises handles via `:func:`_normalise_handle``
so write and read paths agree on handle spelling.
87 changes: 87 additions & 0 deletions taosmd/mentions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
"""Mention index for the A2A bus.

Stores @handle mentions extracted from A2A message bodies and the optional
``recipient`` field, enabling the /a2a/mentions feed and thread-scoped
visibility (anti-bypass rule from taOSmd #211).
"""

from __future__ import annotations

import re
import sqlite3

from taosmd import _db
from taosmd.service import _normalise_handle

_MENTION_RE = re.compile(r'@([a-zA-Z0-9_-]+)')


class MentionStore:
"""Append-only mention index keyed by (mentioned_handle, message_id, ts, thread)."""

def __init__(self, db_path: str) -> None:
self._db_path = db_path
self._conn: sqlite3.Connection | None = None

async def init(self) -> None:
self._conn = _db.connect(self._db_path)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

test

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: Connection leak in init() if executescript() raises

self._conn is assigned before executescript() runs. If that call throws (e.g. SQLite error, disk full, permission denied), the live connection is left open with no cleanup path — the exception propagates and close() is never called.

Wrap the assignment and executescript in a try/except that closes the connection on failure, or reorder so the connection is only assigned after executescript() succeeds.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

self._conn.executescript("""
CREATE TABLE IF NOT EXISTS mentions (
id INTEGER PRIMARY KEY AUTOINCREMENT,
mentioned_handle TEXT NOT NULL,
message_id INTEGER NOT NULL,
ts REAL NOT NULL,
thread TEXT NOT NULL
);
CREATE INDEX IF NOT EXISTS idx_mentions_handle_ts
ON mentions(mentioned_handle, ts);
""")
self._conn.commit()

async def close(self) -> None:
if self._conn:
self._conn.close()
self._conn = None

async def record_mentions(
self,
message_id: int,
body: str,
thread: str,
ts: float,
recipient: str | None = None,
) -> None:
handles: set[str] = set()
for m in _MENTION_RE.finditer(body or ''):
handles.add(_normalise_handle(m.group(1), mint_strip=True))
if recipient:
handles.add(_normalise_handle(recipient, mint_strip=True))

for handle in handles:
self._conn.execute(
"INSERT INTO mentions (mentioned_handle, message_id, ts, thread) VALUES (?, ?, ?, ?)",
(handle, message_id, ts, thread),
)
if handles:
self._conn.commit()

async def get_mentioned_message_ids(
self, reader: str, since: float | None = None, limit: int = 50
) -> list[dict]:
reader = _normalise_handle(reader, mint_strip=True)
query = "SELECT message_id, ts FROM mentions WHERE mentioned_handle = ?"
params: list = [reader]
if since is not None:
query += " AND ts > ?"
params.append(since)
query += " ORDER BY ts ASC LIMIT ?"
params.append(limit)
rows = self._conn.execute(query, params).fetchall()
return [{"message_id": r[0], "ts": r[1]} for r in rows]

async def get_mention_recipients(self, message_id: int) -> list[str]:
rows = self._conn.execute(
"SELECT mentioned_handle FROM mentions WHERE message_id = ?",
(message_id,),
).fetchall()
return [r[0] for r in rows]
5 changes: 3 additions & 2 deletions taosmd/registry_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ def authorize_sender(token: str, claimed_from: str, *, public_key: str,
sub = claims.get("sub")
if not sub:
raise AuthError("token has no 'sub' (canonical_id) claim")
if sub != claimed_from:
from .service import _normalise_handle # noqa: PLC0415
if _normalise_handle(sub) != _normalise_handle(claimed_from):
if human_principal_ids and sub in human_principal_ids:
logger.warning("human principal %r sub/from mismatch: sub=%r, from=%r",
sub, sub, claimed_from)
Expand All @@ -88,7 +89,7 @@ def authorize_sender(token: str, claimed_from: str, *, public_key: str,
)
raise AuthError(f"token sub {sub!r} does not match from {claimed_from!r}")
if not (human_principal_ids and sub in human_principal_ids):
if sub in revoked:
if _normalise_handle(sub) in revoked:
raise AuthError(f"canonical_id {sub!r} is revoked")
if expected_iss is not None and claims.get("iss") != expected_iss:
raise AuthError(f"token iss {claims.get('iss')!r} != expected {expected_iss!r}")
Expand Down
38 changes: 38 additions & 0 deletions taosmd/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

from __future__ import annotations

from pathlib import Path
import hashlib
import json
import logging
Expand All @@ -41,6 +42,27 @@
_remote_cache: dict[tuple[str, str | None], object] = {}


def _normalise_handle(handle: str, *, mint_strip: bool = False) -> str:
"""Slug-match helper for identity comparison on the A2A bus.

This is a slug match, NOT an identity check: two distinct agents that
share a stem will unify under this function. Do not use it for
authorisation decisions that require distinguishing between same-stem
identities.

Strips any leading ``@``, casefolds, and optionally strips a timestamp
mint stamp (``-YYYYMMDD`` or ``-YYYYMMDD-HHMMSS`` suffix) when
``mint_strip=True``. The default is ``mint_strip=False`` because the
``from`` field on bus messages does not carry bare-or-@ twins for the
same canonical, and stripping would collapse install discriminators
such as ``@taOS-agent-1a2b3c4d``.
"""
handle = handle.lstrip("@").casefold()
if mint_strip:
handle = re.sub(r"-\d{8}(-\d{6})?$", "", handle)
return handle


def _get_remote(data_dir=None):
"""Return a cached :class:`~taosmd.remote.RemoteClient` when a server URL
is configured, otherwise ``None`` (use local path).
Expand Down Expand Up @@ -442,6 +464,12 @@ async def a2a_send(
)
stores = await _api._ensure_stores(data_dir)
archive = stores["archive"]
from taosmd.mentions import MentionStore # noqa: PLC0415
db_path = str(Path(data_dir) / "mentions.db") if data_dir else None
mention_store: MentionStore | None = None
if db_path is not None:
mention_store = MentionStore(db_path)
await mention_store.init()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: mention_store connection leaks if archive.record() raises

mention_store is created and init() is called at lines 471–472, but there is no try/finally wrapping the archive.record() call at line 484. If that call raises (e.g. archive storage full, IOError), the exception propagates out of a2a_send without ever reaching the close() calls at lines 499–500, leaking a SQLite connection on every such failure.

Add a try/finally (or async context manager) around the archive write and mention recording to guarantee mention_store.close() runs on all exit paths.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

# Redirect sends to renamed channels: if the target thread has been aliased
# to a new name, route the message to the canonical name instead.
if data_dir is not None:
Expand All @@ -460,6 +488,16 @@ async def a2a_send(
app_id=thread,
summary=body[:200],
)
if mention_store is not None:
await mention_store.record_mentions(
message_id=row_id,
body=body,
thread=thread,
ts=row_id,
recipient=None,
)
if mention_store is not None:
await mention_store.close()
receipt = {"id": row_id, "from": sender, "thread": thread, "reply_to": reply_to}
if refs is not None:
receipt["refs"] = refs
Expand Down
21 changes: 21 additions & 0 deletions tests/test_identity_normalise.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
"""Test that identity comparison uses _normalise_handle."""
from __future__ import annotations

import pytest

from taosmd.service import _normalise_handle


class TestNormaliseHandle:
def test_strips_at_prefix(self):
assert _normalise_handle("@taos-agent") == "taos-agent"

def test_case_fold(self):
assert _normalise_handle("@TaOS-Agent") == "taos-agent"
assert _normalise_handle("TAOS-AGENT") == "taos-agent"

def test_same_handle_no_prefix(self):
assert _normalise_handle("taos-agent") == "taos-agent"

def test_different_handles_still_differ(self):
assert _normalise_handle("taos-agent") != _normalise_handle("other-agent")