chore: drop generated artifacts not tracked on master - #231
Conversation
- Add CONTROLLER_ISS and _is_human_sub to distinguish human principals - authorize_sender now handles human principals (user-* sub) using the same decode_and_verify entry point, skipping revocation and pinning to the controller issuer - /a2a/send derives sender from the verified token sub, not the request body - Presented-but-failing credentials are rejected 403 in both modes for both principal types; missing credential remains the only tolerated class - Humans are skipped for grant checks (no registry identity) - Add unit tests for human auth, cross-claim rejection, and single-chain proof - Add integration tests for human behavior on the live server
📝 WalkthroughWalkthroughRegistry authentication now supports controller-issued human principals. A2A requests use verified token subjects, reject invalid presented credentials in all modes, skip grants for human senders, and persist the verified identity. ChangesHuman Principal Authentication
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant A2AClient
participant http_server
participant RegistryVerifier
participant MessageStore
A2AClient->>http_server: Send message with claimed sender and bearer token
http_server->>RegistryVerifier: Authorize token and claimed sender
RegistryVerifier-->>http_server: Return verified subject and principal type
http_server->>MessageStore: Persist message with verified sender identity
MessageStore-->>http_server: Confirm persistence
http_server-->>A2AClient: Return authentication result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
taosmd/registry_auth.py (1)
162-169: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy liftEager revocation fetch defeats the human bypass guarantee.
authorize()callsself._get_revoked()as a function argument before dispatching toauthorize_sender. Python evaluates this argument eagerly, on every call, regardless of the token's principal type.The docstring for
authorize_senderstates: for human principals (sub starting withuser-): the token must carry a validissmatchinghuman_isswhen set; revocation is not checked (humans are not in the registry).RegistryVerifieralso documents fail-closed semantics: Revocation failures are handled fail-closed where it matters: if the feed has NEVER loaded, :meth:authorizeraises (we cannot prove an agent is unrevoked, so we refuse).Because
_get_revoked()runs unconditionally before the human/agent branch is even reached, a registry revocation feed that has never loaded blocks ALL A2A traffic, including controller-signed human assertions that do not need the revocation feed at all. This contradicts the PR's stated goal of "exclusion of human principals from revocation... checks" and ties human authentication availability to an unrelated registry dependency. The existing tests never exercise this path because the fakerevoked_loaderin test fixtures always succeeds (returns[]).Skip the eager fetch when the caller-asserted identity is already a human
sub(the real signature/sub check insideauthorize_senderstill catches a spoofed claim).🔧 Proposed fix to defer the revocation fetch for human principals
def authorize(self, token: str, claimed_from: str) -> dict: """Authorise a sender; return verified claims or raise AuthError.""" + # Humans don't need the revocation feed at all; skip the eager fetch + # so a registry revocation-feed outage cannot block human traffic. + revoked = set() if _is_human_sub(claimed_from) else self._get_revoked() return authorize_sender( token, claimed_from, - public_key=self._get_pubkey(), revoked=self._get_revoked(), + public_key=self._get_pubkey(), revoked=revoked, expected_iss=self._expected_iss, human_iss=self._human_iss, )🤖 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/registry_auth.py` around lines 162 - 169, Update RegistryVerifier.authorize so it does not eagerly call _get_revoked() when claimed_from identifies a human principal (a sub beginning with “user-”); defer or bypass the revocation fetch for that path while preserving authorize_sender’s signature and claim validation. Continue fetching revocations for agent principals and retain fail-closed behavior when the feed has never loaded.
🤖 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 `@taosmd/http_server.py`:
- Around line 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.
- Around line 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.
- Around line 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.
In `@tests/test_http_server_trust_enforcement.py`:
- Around line 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.
- Around line 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.
---
Outside diff comments:
In `@taosmd/registry_auth.py`:
- Around line 162-169: Update RegistryVerifier.authorize so it does not eagerly
call _get_revoked() when claimed_from identifies a human principal (a sub
beginning with “user-”); defer or bypass the revocation fetch for that path
while preserving authorize_sender’s signature and claim validation. Continue
fetching revocations for agent principals and retain fail-closed behavior when
the feed has never loaded.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 05de5d2a-b4a0-4f64-85d0-06a1d8505286
📒 Files selected for processing (4)
taosmd/http_server.pytaosmd/registry_auth.pytests/test_http_server_trust_enforcement.pytests/test_registry_auth.py
| # 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 "" | ||
|
|
There was a problem hiding this comment.
📐 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.
| 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 | ||
|
|
||
| # 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}" |
There was a problem hiding this comment.
📐 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.
| 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}" |
There was a problem hiding this comment.
🗄️ 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.pyRepository: 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.
| 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 |
There was a problem hiding this comment.
📐 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_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) |
There was a problem hiding this comment.
📐 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 == 403Apply 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
PR Summary by QodoA2A: accept controller-signed human assertions and tighten /a2a/send auth failures
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
13 rules 1. AuthError return skips archiving
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| # 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 |
There was a problem hiding this comment.
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
| # (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 |
There was a problem hiding this comment.
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
|
Closing for rework. The tests are real (37 to 49, they run, they pass) and the crypto choices are sound where they exist: the algorithm is pinned to EdDSA, alg-none is rejected, HMAC confusion is rejected, a foreign key is rejected. But the feature does not do what it says, and it ships a default-mode regression. BLOCKING 1, the feature is decorative. On the no-credential path, a message with from=user-jay and NO token is ACCEPTED in default warn mode, and the sender is stored verbatim as user-jay. Reproduced live against your own fixture: 200, readback shows from user-jay. The PR asserts this as intended behaviour in test_human_missing_token_accepted_in_warn_mode. So this change introduces user-* as a semantically privileged namespace meaning 'a human said this', and simultaneously leaves that namespace claimable by any anonymous client on the LAN. The credential only makes an honest client's message ALSO valid; it gates nothing an attacker has to pass. Every agent reading the bus will treat user-jay as Jay. BLOCKING 2, controller and registry are not a trust boundary. verifier_from_url builds ONE key loader pointed at the registry pubkey path. There is no controller key and no kid selection, so controller-signed versus registry-signed is a string in a claim inside a single signing domain. Your own tests mint both the human assertion and the agent JWT from the same private key, which shows it. Either add a real controller key source, or correct the docstring that says the bus pins this so a registry token cannot spoof a human identity, because today it can. BLOCKING 3, nothing produces these tokens. Nothing in tinyagentos mints a human assertion: no taos-controller issuer anywhere, mint_registry_token hardcodes iss taos-registry, and no user-prefixed subject exists. So no token this PR would accept as a human can currently be created, and the only reachable human path is the credential-free one above. That is the same unwired-feature pattern that sank the earlier taosmd PRs. BLOCKING 4, availability regression in the DEFAULT config, not mentioned in the PR body. authorize() evaluates the revocation feed eagerly before principal dispatch, and invalid-token handling changed to an unconditional 403. Measured on both branches with a valid, granted, unrevoked agent token and the revocation feed unreachable: master returns 200, this branch returns 403. Warn mode is documented as accepting the message with a warning; that is now only true for the missing-token case, so any agent on a stale token breaks the moment this deploys. ALSO: a user-* sub with human_iss unset skips BOTH revocation and issuer pinning, so it must fail closed. And user- is not reserved on the taOS side: slugify('User') yields canonical id user-YYYYMMDD-HHMMSS, which the prefix test classifies as human. Demonstrated: a revoked agent with that shape authorised successfully. MUTATION RESULT worth acting on: disabling the expiry check entirely leaves all 49 tests passing. Expiry is unverified by any test, works only because PyJWT defaults to it, and taOS never mints exp anyway, so every token it issues is an eternal bearer credential. Also none of the 12 new tests catches a forged signature; the one test that does is pre-existing. Adjudication: CodeRabbit's eager-revocation finding is VALID and understated, it affects agents not just humans and is a regression against master. Its finding 4 is partly valid but do not apply the suggested remedy, routing humans through admin auth would widen reach. Kilo is the rate-limited fake red. And please retitle: this is the fifth consecutive taosmd PR wearing 'chore: drop generated artifacts', while actually being a feat that changes bus identity. |
1 participant
Autonomous build of board card tsk-legqtr.
Files:
taosmd/http_server.py | 32 +++++----
taosmd/registry_auth.py | 52 +++++++++++---
tests/test_http_server_trust_enforcement.py | 108 +++++++++++++++++++++++++++-
tests/test_registry_auth.py | 100 ++++++++++++++++++++++++++
4 files changed, 266 insertions(+), 26 deletions(-)
Summary by CodeRabbit
New Features
Bug Fixes