Skip to content

feat(auth): extract admission identity keys and composition - #218

Merged
BegoniaHe merged 6 commits into
masterfrom
feat/admission-identity-keys
Sep 17, 2026
Merged

BegoniaHe merged 6 commits into
masterfrom
feat/admission-identity-keys

Conversation

@BegoniaHe

Copy link
Copy Markdown
Member

Summary

Extract canonical session/sender admission keys and the overlay composition function so later slices can replace the whitelist and add UID rules. Default runtime admission is unchanged: empty sender overlays still follow the current UMO session switches.

Related issue

Fixes #214
Related: #212

Behavior

  • Group session keys are session:{platform_instance}:group:{group_id} whether unique-session is on or off. They never use the rewritten sender_group session id.
  • Private session keys are session:{platform_instance}:private:{peer_id} and can be compared with Subject.im (im:{platform_instance}:{bot}:{sender_id}).
  • compose_admission() implements the parent composition table (unlisted_*, UMO session_enabled / session_blocked / llm_enabled, UID blocked / llm_enabled). Refusal wins; session_blocked overrides a personal LLM exception.
  • SessionServiceManager still reads session_service_config from scope=umo. scope=sender is readable and empty by default. New query: is_sender_blocked.
  • Pipeline order is unchanged. WhitelistCheckStage and id_whitelist stay.

Non-goals

Deleting the whitelist, Dashboard/OpenAPI changes, /user commands, binding persona/KB to senders, changing llm_access, or migrating unique-session /llm disable onto the canonical session key (A2).

Implementation notes

Identity and composition live in astrbot/core/auth/admission.py as pure functions so they do not import AstrMessageEvent (avoids an auth ↔ event cycle). A1 lookups still use event.unified_msg_origin so unique-session /llm disable keeps today's meaning. No docs or OpenAPI change: there is no user-visible behavior shift.

Validation

uv run ruff format astrbot/core/auth/admission.py astrbot/core/star/session_llm_manager.py tests/unit/test_admission.py tests/unit/test_session_llm_manager.py tests/unit/test_waking_check_stage.py tests/unit/test_whitelist_check_stage.py
uv run ruff check astrbot/core/auth/admission.py astrbot/core/star/session_llm_manager.py tests/unit/test_admission.py tests/unit/test_session_llm_manager.py tests/unit/test_waking_check_stage.py tests/unit/test_whitelist_check_stage.py
uv run pyright astrbot/core/auth/admission.py astrbot/core/star/session_llm_manager.py tests/unit/test_admission.py tests/unit/test_session_llm_manager.py
uv run pytest tests/unit/test_admission.py tests/unit/test_session_llm_manager.py tests/unit/test_waking_check_stage.py tests/unit/test_whitelist_check_stage.py tests/unit/test_session_status_check_stage.py tests/unit/test_authorization_service.py tests/unit/test_agent_request_sub_stage.py tests/unit/test_import_boundaries.py tests/unit/test_builtin_command_extensions.py tests/unit/test_turn_routing_current.py tests/unit/test_session_commands.py tests/unit/test_core_import_smoke.py

Ruff format/check passed. Pyright reported 0 errors. The listed pytest files passed (286 tests). make check and the full blocking pytest suite were not run.

Compatibility and risk

No public API, Dashboard protocol, OpenAPI, or docs change. Isolated should_process_llm_request now also reads empty scope=sender overlays; with no UID rows this matches the previous UMO-only LLM switch. session_blocked still does not gate that method (SessionStatusCheck remains the event drop). A written sender llm_enabled would already take effect if one existed.

Checklist

  • A Feature request Issue exists for large work, or this is a small, obvious addition.
  • The change is focused and does not include unrelated refactoring.
  • I added or updated tests, or explained why tests are not practical.
  • User-visible behavior updates both docs/zh/ and docs/en/.
  • OpenAPI, generated client, docs/public/openapi.json, and tests change together when routes or schemas change.
  • No secrets committed. Runtime Python deps update pyproject.toml, requirements.txt, and uv.lock together.
  • I did not restore legacy shims, Python <3.14 fallbacks, or upstream publish/docs URLs as fork artifacts.
  • Breaking API or behavior changes use ! and a BREAKING CHANGE: footer.
  • I will not merge this PR myself. Merge needs a human maintainer review plus a separate AI-assisted review (AI_POLICY.md).
  • AI use follows AI_POLICY.md. Keep exactly one author note below. Do not fabricate the other.

Agent note

Goal: land A1 of #212 — stable session/sender keys plus composition, without changing default admission. Touched astrbot/core/auth/admission.py, astrbot/core/star/session_llm_manager.py, and focused unit tests for composition, unique-session keys, empty-UID equivalence, and the unchanged whitelist matcher. Checks actually run are listed above; full make check / blocking pytest were not. Residual risk: a pre-existing scope=sender row would now affect LLM overlay composition, which A1 wants, but A3 is still the persistence owner. Tools: OpenCode. This write-up is not the required separate AI-assisted review.

Add canonical session/sender keys and overlay composition without changing
default runtime admission. Empty UID overlays still follow the current UMO
session switches. Unique-session rewrites do not change the session key.

Fixes #214
AI-Generated: true
Generated-At: 2026-09-17T15:36:11Z
@BegoniaHe BegoniaHe self-assigned this Sep 17, 2026
Comment thread astrbot/core/auth/admission.py Fixed
Comment thread astrbot/core/auth/admission.py Fixed
Comment thread astrbot/core/auth/admission.py Fixed
Comment thread astrbot/core/auth/admission.py Fixed
Comment thread astrbot/core/auth/admission.py Fixed
Comment thread astrbot/core/auth/admission.py Fixed
Comment thread astrbot/core/auth/admission.py Fixed
Count a session or sender as listed only when an admission field is a
real bool. Garbage keys must not satisfy unlisted-deny. Reuse the typed
service-config helper in LLM and block setters.

Related: #214
AI-Generated: true
Generated-At: 2026-09-17T15:57:36Z
@BegoniaHe

Copy link
Copy Markdown
Member Author

Review follow-up

Reviewed against #214 / #212. Default empty-UID admission, unique-session group keys, and the whitelist matcher look correct. Pushed f79347edf for the issues below.

Fixed

  • listed treated key presence as an allow overlay. {"session_blocked": "yes"} / {"blocked": "yes"} became listed=True while the bool fields stayed default-off. Under unlisted_*=deny that is an allow hole. listed now requires a real bool admission field.
  • LLM / block setters still used or {} on the raw preference. A non-dict row made getters treat the overlay as empty, then the setter crashed. They now reuse _service_config.

Intentionally unchanged

  • session_enabled=False still does not drop admit_event; it is returned on AdmissionDecision for SessionStatusCheck / later slices (recovery-command passthrough is different from session_blocked).
  • should_process_llm_request still ignores blocked flags; SessionStatusCheck remains the event drop. Sender blocked is readable but not wired into the pipeline (A3).
  • CodeQL hits on Protocol ... and admit_event are false positives.

This comment is not an approval.

@xero-team-bot

xero-team-bot Bot commented Sep 17, 2026

Copy link
Copy Markdown

💡 To run commands without mentioning me, run one command with a mention first — @xero-team-bot help — here on this PR. After that, bare commands like yours work without the mention.

Comment thread astrbot/core/auth/admission.py Fixed
Comment thread astrbot/core/auth/admission.py Fixed
Non-dict session_service_config rows still crashed the TTS getter/setter
after LLM and block paths started using _service_config. Invalid
tts_enabled values now default to enabled, matching the bool-only
admission overlay parser.

Related: #214
AI-Generated: true
Generated-At: 2026-09-17T16:25:18Z

@BegoniaHe BegoniaHe left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

AI-assisted review

Reviewed against #214 / #212 and the current branch including 082be4919.

Matches A1

  • Group session keys ignore unique-session session_id rewrites (session:{platform}:group:{group_id}).
  • Private session keys compare with Subject.im on the peer id.
  • compose_admission() covers the parent composition table: refusal wins, sender LLM is more specific, session_blocked overrides a personal LLM exception, garbage overlay values are unlisted.
  • Empty scope=sender rows still follow current UMO session switches. Lookups stay on event.unified_msg_origin.
  • Pipeline order and WhitelistCheckStage / id_whitelist are unchanged.

Fixed in this review

  • TTS getter/setter in SessionServiceManager still used raw or {} after _service_config was extracted for LLM/block. A non-dict session_service_config row crashed TTS and could return a non-bool. They now use the helper, treat invalid tts_enabled as unwritten/enabled, and keep other overlay keys when writing TTS.

Intentionally unchanged

  • session_enabled=False does not drop admit_event; it is exposed on AdmissionDecision for SessionStatusCheck / later slices.
  • should_process_llm_request still ignores blocked flags. Sender blocked is readable, not wired into the pipeline (A3).
  • CodeQL py/ineffectual-statement on Protocol ... and py/uninitialized-local-variable on admit_event are false positives (repo-wide Protocol stub style; admit_event is assigned immediately before use).

Residual risk

  • Group messages with an empty group_id mint session:{platform}:group:unknown. That matches A1's "always use group_id" rule; A2 should not treat unknown as a real room.
  • A pre-existing scope=sender row already affects LLM overlay composition. A1 wants that; A3 still owns persistence.

This is not an approval or merge. Human maintainer review is still required.

Replace Protocol ellipsis stubs with NotImplementedError and stop
naming a compose_admission local after AdmissionDecision.admit_event.

Related: #214
AI-Generated: true
Generated-At: 2026-09-17T16:38:15Z

@BegoniaHe BegoniaHe left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

AI-assisted review

Reviewed against #214 / #212 and the current branch including 8a35a2ecf.

Matches A1

  • Group session keys ignore unique-session session_id rewrites (session:{platform}:group:{group_id}).
  • Private session keys compare with Subject.im on the peer id.
  • compose_admission() covers the parent composition table: refusal wins, sender LLM is more specific, session_blocked overrides a personal LLM exception, garbage overlay values are unlisted.
  • Empty scope=sender rows still follow current UMO session switches. Lookups stay on event.unified_msg_origin.
  • Pipeline order and WhitelistCheckStage / id_whitelist are unchanged.

Fixed in this review

  • CodeQL py/ineffectual-statement on AdmissionEvent Protocol ... stubs. Methods now raise NotImplementedError, matching the existing fix(kb): resolve codeql protocol findings pattern.
  • CodeQL py/uninitialized-local-variable on compose_admission. The local named admit_event collided with AdmissionDecision.admit_event; composition now uses refused and writes the dataclass fields directly.

Intentionally unchanged

  • session_enabled=False does not drop admit_event; it is exposed on AdmissionDecision for SessionStatusCheck / later slices.
  • should_process_llm_request still ignores blocked flags. Sender blocked is readable, not wired into the pipeline (A3).

Residual risk

  • Group messages with an empty group_id mint session:{platform}:group:unknown. That matches A1's "always use group_id" rule; A2 should not treat unknown as a real room.
  • A pre-existing scope=sender row already affects LLM overlay composition. A1 wants that; A3 still owns persistence.

Checks run: ruff format/check and pyright on admission.py; 220 pytest cases across admission, session manager, waking/whitelist/session-status, authorization, and import boundaries. Full make check / blocking pytest were not run.

This is not an approval or merge. Human maintainer review is still required.

Keep the hard-cap invariant (flush before the long debounce) without a
tight processed_at-started_at assertion that flakes on overloaded CI.

Related: #214
AI-Generated: true
Generated-At: 2026-09-17T17:00:42Z
DingTalk unique-session rewrites group session_id to the sender id.
The canonical session key must still use group_id.

Related: #214
AI-Generated: true
Generated-At: 2026-09-17T17:00:42Z
@BegoniaHe

Copy link
Copy Markdown
Member Author

AI-assisted review

Reviewed against #214 / #212 on 2e6d2cb94.

Matches A1

  • Group session keys ignore unique-session session_id rewrites (session:{platform}:group:{group_id}), including DingTalk-style sender-only ids.
  • Private session keys compare with Subject.im on the peer id.
  • compose_admission() covers the parent composition table: refusal wins, sender LLM is more specific, session_blocked overrides a personal LLM exception, garbage overlay values are unlisted.
  • Empty scope=sender rows still follow current UMO session switches. Lookups stay on event.unified_msg_origin.
  • Pipeline order and WhitelistCheckStage / id_whitelist are unchanged.

CI / CodeQL

  • Latest CodeQL Python analysis on 8a35a2ecf was already green. Older in-PR CodeQL comments (py/ineffectual-statement on Protocol ..., py/uninitialized-local-variable on a local named admit_event) are outdated after that commit. Protocol methods keep raise NotImplementedError.
  • macOS blocking pytest failed on test_telegram_media_group_max_wait_is_a_hard_deadline (processed_at - started_at was 1.23s vs < 0.3). Unrelated wall-clock flake: wait_for succeeded and the album flushed, but overloaded CI stretched the 0.15s sleep. Relaxed the test to assert the hard-cap invariant against a 5s debounce without a tight wall-clock bound.

Intentionally unchanged

  • session_enabled=False does not drop admit_event; it is exposed on AdmissionDecision for SessionStatusCheck / later slices.
  • should_process_llm_request still ignores blocked flags. Sender blocked is readable, not wired into the pipeline (A3).

Residual risk

  • Group messages with an empty group_id mint session:{platform}:group:unknown. That matches A1's "always use group_id" rule; A2 should not treat unknown as a real room.
  • A pre-existing scope=sender row already affects LLM overlay composition. A1 wants that; A3 still owns persistence.

Checks run: ruff format/check on the touched tests; 49 pytest cases across admission, session manager, waking/whitelist, and the media-group deadline test. Full make check / blocking pytest were not run locally.

This is not an approval or merge. Human maintainer review is still required.

@BegoniaHe
BegoniaHe merged commit 320f493 into master Sep 17, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] extract admission identity keys and composition resolver

2 participants