SECURITY: notifications read paths ignore user_id — list/list_archived/unread_count return every user's rows and per-id read/dismiss/archive act cross-user (CWE-862, found on #2716 review)

[jaylfc]

CARD TITLE (intent, not commit subject): SECURITY: notifications read paths ignore user_id — list/list_archived/unread_count return every user's rows and per-id read/dismiss/archive act cross-user (CWE-862, found on #2716 review)

Autonomous build of board card tsk-7uxooi.

list, list_archived, unread_count, mark_read, archive, and mark_all_read
now filter on user_id IS NULL OR user_id = ?, so a caller only sees and
can modify their own notifications plus broadcasts. Routes pass the current
user through get_current_user; cross-user mutations return 404.

Red tests added in tests/test_notifications_user_scope.py covering store
and route scoping. Pre-fix: 13 failed (TypeError / missing user_id param).
Post-fix: 88 notification tests pass, including the 13 new red tests.

Docs-Reviewed: routes/notifications.py changes only add user scoping to
existing notification endpoints; docs/agent-coordination.md does not
document these routes so no doc update needed.

Files:
changelog.d/tsk-7uxooi-notifications-user-scope.md | 3 +
tests/test_notifications_user_scope.py | 219 +++++++++++++++++++++
tinyagentos/notifications.py | 80 ++++++--
tinyagentos/routes/notifications.py | 72 +++++--
4 files changed, 344 insertions(+), 30 deletions(-)

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[coderabbitai]

Warning

Review limit reached

Next included review available in 10 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 74562ed8-88cc-4e61-abef-15695278208a

📥 Commits

Reviewing files that changed from the base of the PR and between ae71bb2 and 72893ca.

📒 Files selected for processing (4)

• changelog.d/tsk-7uxooi-notifications-user-scope.md
• tests/test_notifications_user_scope.py
• tinyagentos/notifications.py
• tinyagentos/routes/notifications.py

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gitar-bot]

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

_Gitar

[kilo-code-bot]

SUGGESTION: Unused import — secrets is never referenced in this test module.

Suggested change

	import secrets
	import pytest

---

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

[kilo-code-bot]

SUGGESTION: Weak assertion — assert "1" in resp.text is satisfied by any count containing the digit 1 (e.g. 11, 21, 100). The badge HTML embeds the count both in the body and in data-count, so the route scope test should pin the exact value to make a future cross-user regression impossible to silently mask.

Suggested change

	assert "1" in resp.text
	assert f"data-count='1'" in resp.text and ">1</span>" in resp.text

---

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

[kilo-code-bot]

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	2

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
tests/test_notifications_user_scope.py	1	Unused import secrets
tests/test_notifications_user_scope.py	186	Weak assert "1" in resp.text matches any count containing 1 (e.g. 11, 21) instead of pinning the scoped value

Files Reviewed (4 files)

• changelog.d/tsk-7uxooi-notifications-user-scope.md - 0 issues
• tests/test_notifications_user_scope.py - 2 issues
• tinyagentos/notifications.py - 0 issues
• tinyagentos/routes/notifications.py - 0 issues

Notes

The CWE-862 fix is correct: list, list_archived, unread_count, mark_read, archive, and mark_all_read now consistently filter on (user_id IS NULL OR user_id = ?), and every route mutation passes the session user through get_current_user + _notif_user_id, returning 404 on cross-user attempts. SQL placeholder ordering in list/list_archived/unread_count matches the dynamic condition list, so parameter binding is safe. The pre-existing archive_by_source_ref/cleanup global paths are explicitly documented as intentional and remain unchanged. Pre-existing test infrastructure (tests/conftest.py:client fixture) already supplies a session cookie and CSRF hooks, so the new auth requirement does not regress existing test_notifications.py, test_notifications_mark_all.py, test_taosctl_notifications.py, or test_agent_alias.py cases.

Fix these issues in Kilo Cloud

---

_{Reviewed by minimax-m3:free · Input: 48.4K · Output: 16.2K · Cached: 1.3M}

[jaylfc]

Lead review — blocked, superseded by fix-forward card tsk-47baqy (BASE: exec/tsk-7uxooi). The store-side scoping and the semantics are right and are kept; the route side changes the auth gate the card said not to touch.

L1 (measured): every route now depends on get_current_user, which reads only the taos_session cookie. The middleware also admits the local token as Authorization: Bearer and maps it to the primary user via request.state.user_id (auth_middleware.py:546-573) — that is how taosctl notifications list|read|read-all|mark-all-read|count authenticate (cli/taosctl/client.py:70). Real-app probe with auth.get_local_token(), no cookie, GET /api/notifications and /count: origin/dev 200/200, this branch 401/401. Fix is the repo idiom (routes/event_stream.py:45): resolve getattr(request.state, "user_id", None) and 401 on None.

L2: on origin/dev 11 of the 13 new tests fail with TypeError: list() got an unexpected keyword argument 'user_id'; the three mutation-404 tests never reached their assertion on base because their setup calls the scoped store.list(user_id=...). The red must be the leak, not the signature.

L3 (nit): test_list_archived_returns_own_only picks rows by position from list() while every row shares the same whole-second timestamp.

Details and the DONE-WHEN list are on the card.