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
32 changes: 20 additions & 12 deletions taosmd/http_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,7 @@ def _make_handler(data_dir, runner: _ServiceLoop, verifier=None,
_registry_url,
revoked_token=_registry_admin_token,
expected_iss=registry_auth.REGISTRY_ISS,
human_iss=registry_auth.CONTROLLER_ISS,
)
_grants_verifier = registry_auth.grants_verifier_from_url(
_registry_url,
Expand Down Expand Up @@ -1446,14 +1447,16 @@ def _handle_a2a_send(self) -> None:
# 401/403. In verify-and-warn mode (default) failures are logged as
# a WARNING but the message is accepted, allowing operators to observe
# violations before enabling hard enforcement.
# Human principals (sub starting with user-) are always rejected on
# auth failure regardless of mode: missing credential is the only
# tolerated class during migration.
sender = from_
_is_human = False
if _registry_verifier is not None:
from . import registry_auth # noqa: PLC0415 - optional path
auth = self.headers.get("Authorization", "")
token = auth[len("Bearer "):].strip() if auth.startswith("Bearer ") else ""

Comment on lines +1450 to 1459

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the comment to match the generalized rejection policy.

The comment states: Human principals (sub starting with user-) are always rejected on auth failure regardless of mode: missing credential is the only tolerated class during migration. The actual implementation, per the comment further down, applies this rule to every principal type: Presented-but-failing credentials are always rejected (both modes), regardless of principal type. Missing credential is the only class tolerated during migration.

Reword the comment at Line 1450 so it does not imply the always-reject rule is human-specific. A future maintainer reading only this comment could reintroduce a warn-mode tolerance for agents with invalid tokens.

🤖 Prompt for 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.

In `@taosmd/http_server.py` around lines 1450 - 1459, Update the comment above
sender authentication in the registry verifier path to state that
presented-but-failing credentials are always rejected in every mode for all
principal types, while only missing credentials may be tolerated during
migration. Remove the human-specific qualification and preserve the surrounding
implementation.

# Compute warn_reason (None = auth passed) and the status/message
# to use in enforce mode. We collect these without returning early
# so the enforce vs. warn decision is made in one place below.
warn_reason: str | None = None
_reject_status: int = 403
_reject_msg: str = ""
Expand All @@ -1464,19 +1467,24 @@ def _handle_a2a_send(self) -> None:
_reject_msg = "registry auth: Bearer token required"
else:
try:
_registry_verifier.authorize(token, from_)
claims = _registry_verifier.authorize(token, from_)
sender = claims["sub"]
_is_human = registry_auth._is_human_sub(sender)
except registry_auth.AuthError as exc:
warn_reason = str(exc)
_reject_status = 403
_reject_msg = f"registry auth: {exc}"
# Presented-but-failing credentials are always rejected
# (both modes), regardless of principal type. Missing
# credential is the only class tolerated during migration.
self._send_json(403, {"error": f"registry auth: {exc}"})
return
Comment on lines +1474 to +1478

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Autherror return skips archiving 📘 Rule violation ☼ Reliability

_handle_a2a_send() now returns immediately on presented-but-invalid credentials, so the request is
rejected without any archive.record() call. This violates the requirement to archive interactions
on both success and failure paths (including errors).
Agent Prompt
## Issue description
The HTTP entrypoint `_handle_a2a_send()` returns early on `registry_auth.AuthError`, sending a 403 response without archiving the failed interaction.

## Issue Context
The success path archives via `service.a2a_send()` (which calls `archive.record(...)`), but the new early-return failure path bypasses that entirely. Compliance requires archiving error/failure turns as well.

## Fix Focus Areas
- taosmd/http_server.py[1469-1505]
- taosmd/service.py[344-403]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +1475 to +1478

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Warn-mode docs stale 🐞 Bug ◔ Observability

taosmd/http_server.py documents verify-and-warn mode as logging auth failures and still accepting
messages, but _handle_a2a_send now returns 403 immediately for presented-but-invalid credentials
(skipping the warn/enforce decision and any warning log). This makes operator guidance inaccurate
and removes warn-mode visibility into invalid credential attempts.
Agent Prompt
## Issue description
`POST /a2a/send` verify-and-warn semantics are now inconsistent with the module’s documentation/comments: presented-but-invalid credentials are rejected with 403 even when `a2a_auth_enforce=false`, and this path returns before the warning log branch.

## Issue Context
This appears intentional per the updated tests (invalid token rejected regardless of mode), but the top-of-file “Security note” still describes verify-and-warn as accepting failures and logging a warning, which is no longer true for invalid tokens (and humans also skip grant checks).

## Fix Focus Areas
- Update the `http_server.py` Security note / comments to accurately describe current behavior (what is accepted in warn mode vs rejected in all modes).
- Decide and implement expected observability for rejected invalid credentials (e.g., emit a `logger.warning(...)` or structured auth-failure log before returning 403, without logging the token).
- Optionally add/adjust tests to assert the intended logging behavior for invalid-token rejections in warn mode.

### Files/lines to change
- taosmd/http_server.py[24-44]
- taosmd/http_server.py[1444-1501]
- tests/test_http_server_trust_enforcement.py[187-203]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


# Grant check: token proves identity; grant proves permission.
if warn_reason is None and _grants_verifier is not None:
# Humans are not in the registry, so they have no grants.
if warn_reason is None and _grants_verifier is not None and not _is_human:
try:
if not _grants_verifier.has_grant(from_):
if not _grants_verifier.has_grant(sender):
warn_reason = "no a2a_send grant"
_reject_status = 403
_reject_msg = f"registry auth: no active grant for {from_!r}"
_reject_msg = f"registry auth: no active grant for {sender!r}"
Comment on lines +1470 to +1487

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Avoid calling a module-private helper across module boundaries.

registry_auth._is_human_sub is underscore-prefixed, signaling it is module-internal. http_server.py calls it directly at Line 1472. Export it as a public function (drop the leading underscore) since it is now part of the cross-module contract for identifying human principals.

♻️ Proposed rename to a public helper
-def _is_human_sub(sub: str) -> bool:
+def is_human_sub(sub: str) -> bool:
     """Return True if the sub is a human canonical id (user-* convention)."""
     return sub.startswith("user-")

Update the internal call site in authorize_sender and the external call in http_server.py (registry_auth._is_human_sub(sender)registry_auth.is_human_sub(sender)) accordingly.

🤖 Prompt for 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.

In `@taosmd/http_server.py` around lines 1470 - 1487, Rename the module-private
helper _is_human_sub to the public is_human_sub in registry_auth, then update
both authorize_sender and the http_server.py call site to use the new public
symbol. Preserve the helper’s existing behavior and all other authorization
logic.

Comment on lines +1482 to +1487

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check whether human principals (user-* subjects) are expected to reach
# endpoints gated by _apply_token_binding, and whether has_grant is exempted for them.
rg -n -C5 '_apply_token_binding|has_grant\(' taosmd/http_server.py

Repository: jaylfc/taosmd

Length of output: 9483


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== http_server.py relevant token/human binding sections =="
sed -n '730,875p' taosmd/http_server.py | cat -n | sed 's/^/730+/'
echo

echo "== occurrences of _is_human_sub / user- / human principals =="
rg -n -C3 '_is_human_sub|user-_|is_human|human|_token_project|_apply_token_binding' taosmd/http_server.py
echo

echo "== route handlers mentioning /ingest /search /tasks =="
rg -n -C4 '"/(ingest|search|tasks|tasks/|ingest_batch)|ingest_batch|task_list|task_ready|task_prime|task_edges|_handle_task' taosmd/http_server.py | sed -n '1,240p'

Repository: jaylfc/taosmd

Length of output: 27628


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== registry_auth.py outline =="
ast-grep outline taosmd/registry_auth.py --view expanded || true
echo

echo "== registry_auth.py relevant constants and _is_human_sub implementation =="
rg -n -C8 'REGISTRY_ISS|CONTROLLER_ISS|_is_human_sub|has_grant|verify|decode' taosmd/registry_auth.py

echo
echo "== route auth gate coverage =="
sed -n '888,1002p' taosmd/http_server.py | cat -n | sed 's/^/888+/'

Repository: jaylfc/taosmd

Length of output: 16532


Keep human principals out of registry-gated data endpoints.

Human principals are allowed on A2A because they are not registry grant members. Route them through admin-only auth instead of letting controller human tokens reach /ingest, /search, or /tasks/*, because _apply_token_binding will reject those requests with no active grant when the grant feed is present and the principal is human. Do not apply the A2A human exemption to these data endpoints unless each endpoint is explicitly designed for human users.

🤖 Prompt for 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.

In `@taosmd/http_server.py` around lines 1482 - 1487, Update the auth branching
around _grants_verifier.has_grant and the _is_human check so human principals
are excluded from registry-gated data endpoints (/ingest, /search, and
/tasks/*). Route human requests through the existing admin-only authentication
path, while preserving the A2A-specific human exemption and grant validation for
non-human principals.

except registry_auth.AuthError as exc:
warn_reason = str(exc)
_reject_status = 403
Expand All @@ -1489,11 +1497,11 @@ def _handle_a2a_send(self) -> None:
return
logger.warning(
"a2a verify-and-warn: accepting unverified post from %r: %s",
from_, warn_reason,
sender, warn_reason,
)
result = runner.run(
service.a2a_send(
sender=from_, body=body_text,
sender=sender, body=body_text,
thread=thread, reply_to=reply_to,
refs=refs, blocks=blocks,
data_dir=data_dir,
Expand Down
52 changes: 41 additions & 11 deletions taosmd/registry_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,20 @@
# pins this so a token from any other issuer is rejected.
REGISTRY_ISS = "taos-registry"

# The literal ``iss`` the controller mints into human assertions. The bus pins
# this so a registry token cannot spoof a human identity (and vice versa).
CONTROLLER_ISS = "taos-controller"


class AuthError(Exception):
"""Raised when a token fails verification or the auth policy."""


def _is_human_sub(sub: str) -> bool:
"""Return True if the sub is a human canonical id (user-* convention)."""
return sub.startswith("user-")


def _require_jwt():
try:
import jwt # noqa: PLC0415
Expand All @@ -58,25 +67,37 @@ def decode_and_verify(token: str, public_key: str) -> dict:


def authorize_sender(token: str, claimed_from: str, *, public_key: str,
revoked: set[str], expected_iss: str | None = None) -> dict:
revoked: set[str], expected_iss: str | None = None,
human_iss: str | None = None) -> dict:
"""Authorise a bus sender. Returns the verified claims or raises AuthError.

Policy (after the EdDSA signature check):
* the token must carry a ``sub`` (the agent canonical_id);
* the token must carry a ``sub`` (the agent or human canonical_id);
* ``sub`` must equal the message ``from`` (no impersonation);
* ``sub`` must not be in the registry revocation set;
* when ``expected_iss`` is set, ``iss`` must match it (issuer pinning).
* for human principals (sub starting with ``user-``):
- the token must carry a valid ``iss`` matching ``human_iss`` when set;
- revocation is not checked (humans are not in the registry);
* for agent principals:
- ``sub`` must not be in the registry revocation set;
- when ``expected_iss`` is set, ``iss`` must match it (issuer pinning).
"""
claims = decode_and_verify(token, public_key)
sub = claims.get("sub")
if not sub:
raise AuthError("token has no 'sub' (canonical_id) claim")
if sub != claimed_from:
raise AuthError(f"token sub {sub!r} does not match from {claimed_from!r}")
if 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}")
if _is_human_sub(sub):
if human_iss is not None and claims.get("iss") != human_iss:
raise AuthError(
f"human token iss {claims.get('iss')!r} != expected {human_iss!r}"
)
# Humans are not in the registry: skip revocation check.
else:
if 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}")
return claims


Expand All @@ -100,12 +121,14 @@ class RegistryVerifier:

def __init__(self, *, pubkey_loader, revoked_loader,
refresh_interval: float = 300.0, clock=time.time,
expected_iss: str | None = None):
expected_iss: str | None = None,
human_iss: str | None = None):
self._pubkey_loader = pubkey_loader
self._revoked_loader = revoked_loader
self._refresh_interval = refresh_interval
self._clock = clock
self._expected_iss = expected_iss
self._human_iss = human_iss
self._pubkey: str | None = None
self._revoked: set[str] = set()
self._revoked_fetched_at: float | None = None
Expand Down Expand Up @@ -142,6 +165,7 @@ def authorize(self, token: str, claimed_from: str) -> dict:
token, claimed_from,
public_key=self._get_pubkey(), revoked=self._get_revoked(),
expected_iss=self._expected_iss,
human_iss=self._human_iss,
)


Expand Down Expand Up @@ -337,7 +361,8 @@ def grants_verifier_from_url(base_url: str, *, refresh_interval: float = 300.0,
def verifier_from_url(base_url: str, *, refresh_interval: float = 300.0,
opener=_http_get, clock=time.time,
expected_iss: str | None = REGISTRY_ISS,
revoked_token: str | None = None) -> "RegistryVerifier":
revoked_token: str | None = None,
human_iss: str | None = CONTROLLER_ISS) -> "RegistryVerifier":
"""Build a :class:`RegistryVerifier` that fetches from a registry base URL.

The HTTP getter is injectable (``opener``) so callers/tests can supply
Expand All @@ -346,11 +371,16 @@ def verifier_from_url(base_url: str, *, refresh_interval: float = 300.0,
``revoked_token`` is the taOS local/admin token sent as a Bearer header on
the revoked-feed poll (the #710 contract moved it behind admin auth). The
pubkey endpoint stays public and is fetched without a token.

``human_iss`` is the expected ``iss`` for human principal assertions. When
set, any token whose ``sub`` starts with ``user-`` must carry this issuer.
"""
base = base_url.rstrip("/")
return RegistryVerifier(
pubkey_loader=lambda: parse_pubkey_response(opener(base + PUBKEY_PATH)),
revoked_loader=lambda: parse_revoked_response(
opener(base + REVOKED_PATH, token=revoked_token)),
refresh_interval=refresh_interval, clock=clock, expected_iss=expected_iss,
refresh_interval=refresh_interval, clock=clock,
expected_iss=expected_iss,
human_iss=human_iss,
)
108 changes: 105 additions & 3 deletions tests/test_http_server_trust_enforcement.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,28 @@ def fake_opener(url, timeout=5.0, token=None):
return verifier, gv


def _make_human_verifier():
"""Build (registry_verifier, grants_verifier) pair configured for humans."""
def fake_opener(url, timeout=5.0, token=None):
if url.endswith(registry_auth.PUBKEY_PATH):
return json.dumps({"pubkey": PUB_PEM})
if url.endswith(registry_auth.REVOKED_PATH):
return json.dumps([])
if url.endswith(registry_auth.GRANTS_PATH):
return json.dumps({"grants": []})
raise ValueError(f"unexpected url: {url}")

verifier = registry_auth.verifier_from_url(
"http://reg.test", opener=fake_opener,
expected_iss=registry_auth.REGISTRY_ISS,
human_iss=registry_auth.CONTROLLER_ISS,
)
gv = registry_auth.grants_verifier_from_url(
"http://reg.test", opener=fake_opener,
)
return verifier, gv


@pytest.fixture
def warn_server(tmp_path, monkeypatch):
"""Server with verifiers wired in but a2a_auth_enforce NOT set (default=False).
Expand Down Expand Up @@ -173,12 +195,11 @@ def test_warn_no_token_accepted(warn_server, caplog):


def test_warn_invalid_token_accepted(warn_server, caplog):
"""Invalid token: message accepted and warning logged in warn mode."""
"""Invalid token: message rejected with 403 regardless of mode."""
import logging
with caplog.at_level(logging.WARNING, logger="taosmd.http_server"):
status, body = _post_send(warn_server, "any-agent", "hello", token="not-a-jwt")
assert status == 200, body
assert any("verify-and-warn" in r.message for r in caplog.records)
assert status == 403, body
Comment on lines 197 to +202

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Rename the test to match its new assertion.

The function is named test_warn_invalid_token_accepted, but the body now asserts rejection: """Invalid token: message rejected with 403 regardless of mode.""" Rename it (e.g. test_warn_invalid_token_rejected) so the test name reflects the current behavior and does not mislead readers relying on test names to understand policy.

🧰 Tools
🪛 Ruff (0.16.0)

[error] 201-201: Possible hardcoded password assigned to argument: "token"

(S106)

🤖 Prompt for 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.

In `@tests/test_http_server_trust_enforcement.py` around lines 197 - 202, Rename
the test function test_warn_invalid_token_accepted to reflect that invalid
tokens are rejected with HTTP 403, such as test_warn_invalid_token_rejected;
leave the test body and assertions unchanged.



def test_warn_valid_token_no_grant_accepted(warn_server, caplog):
Expand Down Expand Up @@ -330,3 +351,84 @@ def test_api_still_up_when_dashboard_hidden(tmp_path, monkeypatch):
finally:
httpd.shutdown()
httpd.service_loop.close()


# ---------------------------------------------------------------------------
# Human principal support (unified-chat slice 3)
# ---------------------------------------------------------------------------

@pytest.fixture
def human_warn_server(tmp_path, monkeypatch):
"""Server with human-capable verifier, a2a_auth_enforce NOT set (default).

Auth failures are rejected 403 in both modes (human policy).
"""
data_dir = tmp_path / "data"
data_dir.mkdir()
monkeypatch.setattr(taosmd_api, "_stores_cache", {})

verifier, gv = _make_human_verifier()
httpd = http_server.make_server(
"127.0.0.1", 0, data_dir=str(data_dir),
verifier=verifier, grants_verifier=gv,
)
httpd.service_loop.run(taosmd_api._ensure_stores(str(data_dir)))
host, port = httpd.server_address[:2]
thread = threading.Thread(target=httpd.serve_forever, daemon=True)
thread.start()
try:
yield f"http://{host}:{port}"
finally:
httpd.shutdown()
httpd.service_loop.close()


def test_human_assertion_sub_mismatch_rejected_in_warn_mode(human_warn_server):
"""Human assertion whose sub does not match from is rejected 403 even in
warn mode (fail-first: on master this would be 200)."""
token = pyjwt.encode(
{"sub": "user-123", "iss": registry_auth.CONTROLLER_ISS},
PRIV_PEM, algorithm="EdDSA",
)
status, body = _post_send(human_warn_server, "user-456", "hello", token=token)
assert status == 403


def test_human_assertion_sub_match_accredited(human_warn_server):
"""Valid human assertion with matching sub is accepted."""
token = pyjwt.encode(
{"sub": "user-123", "iss": registry_auth.CONTROLLER_ISS},
PRIV_PEM, algorithm="EdDSA",
)
status, body = _post_send(human_warn_server, "user-123", "hello", token=token)
assert status == 200, body


def test_agent_jwt_claiming_human_id_rejected_in_warn_mode(human_warn_server):
"""An agent JWT (iss=taos-registry) with a user-* sub is rejected 403."""
token = pyjwt.encode(
{"sub": "user-123", "iss": registry_auth.REGISTRY_ISS},
PRIV_PEM, algorithm="EdDSA",
)
status, body = _post_send(human_warn_server, "user-123", "hello", token=token)
assert status == 403


def test_human_assertion_claiming_agent_id_rejected_in_warn_mode(human_warn_server):
"""A human assertion (iss=taos-controller) with an agent sub is rejected 403."""
token = pyjwt.encode(
{"sub": "agent-1", "iss": registry_auth.CONTROLLER_ISS},
PRIV_PEM, algorithm="EdDSA",
)
status, body = _post_send(human_warn_server, "agent-1", "hello", token=token)
assert status == 403


def test_human_missing_token_accepted_in_warn_mode(human_warn_server, caplog):
"""Missing token for a human principal is accepted with warning in warn mode."""
import logging
with caplog.at_level(logging.WARNING, logger="taosmd.http_server"):
status, body = _post_send(human_warn_server, "user-123", "hello")
assert status == 200, body
assert any("verify-and-warn" in r.message and "missing Bearer token" in r.message
for r in caplog.records)
Comment on lines +386 to +434

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Logic is correct; clean up unused unpacked variables.

The four new tests correctly exercise sub mismatch, matching-sub acceptance, cross-principal issuer rejection in both directions, and missing-token tolerance. Static analysis flags body as unused at Lines 393, 413, and 423 (unlike the sibling assertions at Lines 404 and 432, which pass body as the assert message). Prefix the unused unpacking with an underscore.

🧹 Proposed fix for unused unpacked variables
-    status, body = _post_send(human_warn_server, "user-456", "hello", token=token)
+    status, _body = _post_send(human_warn_server, "user-456", "hello", token=token)
     assert status == 403

Apply the same change at Lines 413 and 423.

🧰 Tools
🪛 Ruff (0.16.0)

[warning] 393-393: Unpacked variable body is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


[warning] 413-413: Unpacked variable body is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


[warning] 423-423: Unpacked variable body is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)

🤖 Prompt for 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.

In `@tests/test_http_server_trust_enforcement.py` around lines 386 - 434, Prefix
the unused response-body unpacking variable with an underscore in
test_human_assertion_sub_mismatch_rejected_in_warn_mode,
test_agent_jwt_claiming_human_id_rejected_in_warn_mode, and
test_human_assertion_claiming_agent_id_rejected_in_warn_mode, while leaving the
status assertions unchanged.

Source: Linters/SAST tools

Loading