Conversation
(cherry picked from commit e074a91)
Co-authored-by: Julia Johannesen <julia@insertdomain.name> (cherry picked from commit b5d3996)
Co-authored-by: Julia Johannesen <197614925+juliajohannesen@users.noreply.github.com> (cherry picked from commit a07dc58)
* Tighten security in `HashtagChannel` * Add isNoteVisibleForMe in stream channel Co-Authored-By: Julia Johannesen <julia@insertdomain.name> * Tighten note visibility checks in WebSocket (No.1) * refactor * Fix main channel Co-Authored-By: Julia Johannesen <julia@insertdomain.name> * fix typo * fix missing lockdown (requireSigninToViewContents) checks * fix(backend): streamingでのロックダウン挙動修正 * fix: 引用リノートを無条件で隠していた問題を修正 * fix: 引用リノートを単純にリノート場合に内容が見えることがある問題を修正 * refac * fix * fix * fix * Update docs --------- Co-authored-by: Julia Johannesen <julia@insertdomain.name> Co-authored-by: KanariKanaru <93921745+kanarikanaru@users.noreply.github.com> (cherry picked from commit b361a10)
* Tighten security on channels * Fix main channel * add comments, improve typing * fix indent * fix: missing membership checks in chat-room * remove unnecessary check in chat-user * fix * refactor: use exists * fix --------- Co-authored-by: Julia Johannesen <julia@insertdomain.name> (cherry picked from commit 06e7450)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthroughストリーミングの隠蔽判定を新規 NoteStreamingHidingService に集約して各チャネルへ DI。Connection のチャネル管理を配列→Map に変更し非同期化。publishNoteStream の第1引数を note オブジェクトへ変更。複数エンドポイントでドライブファイル所有権を厳格化。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Connection
participant Channel
participant Hiding as NoteStreamingHidingService
participant Entity as NoteEntityService
participant Subscriber
Client->>Connection: connectChannel(params)
Connection->>Channel: init(params)
Channel->>Hiding: processHiding(note, meId)
alt shouldSkip == true
Hiding-->>Channel: { shouldSkip: true }
Channel-->>Connection: skip delivery
else shouldSkip == false
Hiding->>Entity: (必要なら) hideNote(packedNote)
Entity-->>Hiding: mutated note
Hiding-->>Channel: { shouldSkip: false, hiddenLayers }
Channel->>Subscriber: deliver(note)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Documentation Updates 1 document(s) were updated by changes in this PR: Misskeyの一元化されたNote取得フローと重複防止メカニズムView Changes@@ -2,7 +2,9 @@
## 概要
-Misskeyは、一元化されたバックエンドサービスとインテリジェントなフロントエンドメカニズムを組み合わせた高度なNote取得アーキテクチャを実装しています。このアーキテクチャは、重複取得の防止、障害の適切な処理、UIの応答性維持を実現します。
+Misskeyは、一元化されたバックエンドサービスとインテリジェントなフロントエンドメカニズムを組み合わせた高度なNote取得アーキテクチャを実装しています。このアーキテクチャは、重複取得の防止、障害の適切な処理、UIの応答性維持、そしてストリーミング配信時のNote可視性制御を実現します。
+
+**セキュリティに関する注記:** このドキュメントで説明されているアーキテクチャの一部は、CVE-2026-28431およびGHSA-r33c-qg3g-v9crで報告されたNote可視性とストリーミングに関するセキュリティ問題に対応するために強化されています。
## 1. 一元化されたNote取得アーキテクチャ
@@ -324,16 +326,321 @@
### 4.5 WebSocketベースのリアルタイムストリーミング
-[MkTimelineはWebSocket接続を確立](https://github.com/MisskeyIO/misskey/blob/922adb1e4e51ed5c71d63430b8f3a3f3b756fee5/packages/frontend/src/components/MkTimeline.vue#L244-L326)し、Noteが作成されるとそれらが非同期にプッシュされます。ストリーミング接続は**最小化されたNoteデータ**（IDと基本的なメタデータのみ）を送信し、完全なNoteデータは必要なときにのみ遅延取得されます。このアプローチは以下を実現します:
-
-- 帯域幅消費の削減
-- 処理オーバーヘッドの最小化
-- Noteデータのプログレッシブエンハンスメント
-- 高トラフィック期間中のUI応答性の維持
-
-## 5. これらのパターンがUI応答性とロード安定性を向上させる仕組み
-
-### 5.1 UI応答性の利点
+[MkTimelineはWebSocket接続を確立](https://github.com/MisskeyIO/misskey/blob/922adb1e4e51ed5c71d63430b8f3a3f3b756fee5/packages/frontend/src/components/MkTimeline.vue#L244-L326)し、Noteが作成されるとそれらが非同期にプッシュされます。ストリーミング接続は**最小化されたNoteデータ**（IDと基本的なメタデータのみ）を送信し、完全なNoteデータは必要なときにのみ遅延取得されます。
+
+#### ストリーミングイベントのメタデータ拡張
+
+[GlobalEventService.publishNoteStream()](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/core/GlobalEventService.ts)は、Noteストリーミングイベントに可視性フィルタリングのための追加メタデータを含めるように拡張されています:
+
+```typescript
+public publishNoteStream(note: MiNote, type: K, value?: NoteEventTypes[K]): void {
+ this.publish(`noteStream:${note.id}`, type, {
+ id: note.id,
+ userId: note.userId,
+ visibility: note.visibility,
+ visibleUserIds: note.visibleUserIds,
+ mentions: note.mentions,
+ replyUserId: note.replyUserId,
+ body: value,
+ });
+}
+```
+
+これにより、以下が実現されます:
+- **早期フィルタリング**: 追加のデータベースクエリなしでクライアント側で可視性判定が可能
+- **帯域幅最適化**: 不適切なNoteをクライアントに送信する前にフィルタリング
+- **コンテキスト保持**: リプライとメンションの関係を効率的に追跡
+- **処理オーバーヘッドの最小化**: Noteデータのプログレッシブエンハンスメント
+- **高トラフィック期間中のUI応答性の維持**
+
+## 5. ストリーミング配信時のNote可視性制御
+
+### 5.1 NoteEntityServiceのリファクタリング
+
+[NoteEntityService](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/core/entities/NoteEntityService.ts)は、Note可視性判定ロジックをよりモジュール化され再利用可能な形にリファクタリングしました。
+
+#### 可視性判定メソッドの分離
+
+以前は`hideNote()`メソッドが判定と変更の両方を行っていましたが、現在は以下のように分離されています:
+
+**[shouldHideNote()](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/core/entities/NoteEntityService.ts)**: Noteを隠すべきかを判定するpublicメソッド
+```typescript
+public async shouldHideNote(packedNote: Packed<'Note'>, meId: MiUser['id'] | null): Promise<boolean> {
+ if (meId === packedNote.userId) return false;
+
+ if (this.isSigninOrTimeHidden(packedNote, meId)) {
+ return true;
+ }
+
+ if (packedNote.visibility === 'specified' && !this.isSpecifiedVisibleTo(packedNote, meId)) {
+ return true;
+ }
+
+ if (packedNote.visibility === 'followers' && !(await this.isFollowerVisibleTo(packedNote, meId))) {
+ return true;
+ }
+
+ return false;
+}
+```
+
+**[hideNote()](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/core/entities/NoteEntityService.ts)**: Note内容を隠すpublicメソッド（変更のみ実行）
+```typescript
+public hideNote(packedNote: Packed<'Note'>): void {
+ packedNote.visibleUserIds = undefined;
+ packedNote.fileIds = [];
+ packedNote.files = [];
+ packedNote.text = null;
+ packedNote.poll = undefined;
+ packedNote.cw = null;
+ packedNote.isHidden = true;
+}
+```
+
+#### ヘルパーメソッドの導入
+
+可視性ロジックを整理するために、以下のヘルパーメソッドが追加されました:
+
+- **[isSpecifiedVisibleTo()](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/core/entities/NoteEntityService.ts)**: 指名可視性（visibility: specified）の判定
+- **[isSigninOrTimeHidden()](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/core/entities/NoteEntityService.ts)**: サインイン要件と時間制限の判定
+- **[isFollowerVisibleTo()](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/core/entities/NoteEntityService.ts)**: フォロワー可視性（visibility: followers）の判定
+
+#### パフォーマンス最適化
+
+`isFollowerVisibleTo()`メソッドは、データベースクエリの代わりに`userFollowingsCache`を使用してフォロー関係をチェックするように最適化されました:
+
+```typescript
+const followings = await this.cacheService.userFollowingsCache.fetch(meId);
+if (Object.hasOwn(followings, packedNote.userId)) {
+ return true;
+}
+```
+
+これにより、ストリーミング配信時のフォロワー可視性チェックのパフォーマンスが大幅に向上します。
+
+#### packed noteへのreplyUserIdフィールド追加
+
+Noteエンティティは[replyUserIdフィールド](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/core/entities/NoteEntityService.ts)を含むようになり、リプライの文脈をより効率的に判定できます:
+
+```typescript
+replyUserId: note.reply?.userId ?? note.replyUserId,
+```
+
+### 5.2 NoteStreamingHidingService
+
+[NoteStreamingHidingService](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/server/api/stream/NoteStreamingHidingService.ts)は、ストリーミング配信時のNote可視性を処理する新しい専用サービスです。このサービスは、多層リノート構造における可視性ルールに基づいてNoteを隠すべきか、またはスキップすべきかを判定します。
+
+#### 3層の隠蔽システム
+
+サービスは、ネストされたリノート構造に対応する3層の隠蔽レイヤーを実装しています:
+
+1. **'note'レイヤー**: トップレベルのNote自体
+2. **'renote'レイヤー**: note.renote（1階層目のリノート）
+3. **'renoteRenote'レイヤー**: note.renote.renote（2階層目のリノート）
+
+```typescript
+type HiddenLayer = 'note' | 'renote' | 'renoteRenote';
+```
+
+#### shouldHide()メソッド: 多層可視性評価
+
+[shouldHide()メソッド](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/server/api/stream/NoteStreamingHidingService.ts)は、ネストされたリノート階層全体にわたって可視性を評価します:
+
+```typescript
+public async shouldHide(
+ note: Packed<'Note'>,
+ meId: MiUser['id'] | null,
+): Promise<LockdownCheckResult>
+```
+
+**判定ロジック:**
+
+1. **1階層目（note自体）**:
+ - 可視性違反かつ純粋リノート → **ストリームをスキップ**
+ - 可視性違反かつ引用リノート → `'note'`レイヤーを隠す
+ - 可視性違反かつ通常Note → `'note'`レイヤーを隠す
+
+2. **2階層目（note.renote）**:
+ - noteが引用リノートで、renoteが可視性違反 → `'renote'`レイヤーを隠す
+ - noteが純粋リノートで、renoteが可視性違反 → **ストリームをスキップ**
+
+3. **3階層目（note.renote.renote）**:
+ - note.renoteが引用リノートで、renote.renoteが可視性違反 → `'renoteRenote'`レイヤーを隠す
+ - note.renoteが純粋リノートで、renote.renoteが可視性違反 → **ストリームをスキップ**
+
+**純粋リノート vs. 引用リノートの処理の違い:**
+- **純粋リノート**: 可視性違反がある場合、Note全体をスキップ（配信しない）
+- **引用リノート**: 可視性違反がある場合、違反部分のコンテンツを隠すが配信は継続
+
+これにより、ユーザーは引用リノートの存在を知ることができますが、閲覧権限のない引用元コンテンツは保護されます。
+
+#### applyHiding()メソッド: コンテンツ変更
+
+[applyHiding()メソッド](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/server/api/stream/NoteStreamingHidingService.ts)は、判定された隠蔽レイヤーに基づいてNoteオブジェクトを変更します:
+
+```typescript
+public applyHiding(
+ note: Packed<'Note'>,
+ hiddenLayers: Set<HiddenLayer>,
+): void {
+ if (hiddenLayers.has('note')) {
+ this.noteEntityService.hideNote(note);
+ }
+ if (hiddenLayers.has('renote') && note.renote) {
+ this.noteEntityService.hideNote(note.renote);
+ }
+ if (hiddenLayers.has('renoteRenote') && note.renote && note.renote.renote) {
+ this.noteEntityService.hideNote(note.renote.renote);
+ }
+}
+```
+
+このメソッドは渡された`note`オブジェクトを直接変更し、各レイヤーで`NoteEntityService.hideNote()`を呼び出します。
+
+#### processHiding()メソッド: 統合フロー
+
+[processHiding()メソッド](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/server/api/stream/NoteStreamingHidingService.ts)は、判定と適用を組み合わせた便利なメソッドです:
+
+```typescript
+public async processHiding(
+ note: Packed<'Note'>,
+ meId: MiUser['id'] | null,
+): Promise<{ shouldSkip: boolean }> {
+ const result = await this.shouldHide(note, meId);
+ if (result.shouldSkip) {
+ return { shouldSkip: true };
+ }
+ this.applyHiding(note, result.hiddenLayers);
+ return { shouldSkip: false };
+}
+```
+
+これは、ストリーミングチャネルでの使用に最適化されたワンステップインターフェースを提供します。
+
+### 5.3 ストリーミングチャネル接続の強化
+
+#### 非同期初期化とブーリアン戻り値
+
+[Channel.init()メソッド](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/server/api/stream/channel.ts)のシグネチャが拡張され、接続の有効性を示すブーリアン値を返せるようになりました:
+
+```typescript
+public abstract init(params: JsonObject): Awaitable<void | boolean>;
+```
+
+**戻り値の意味:**
+- `void` / `Promise<void>`: チェックなし（常に接続を許可）
+- `true` / `Promise<true>`: 接続可能
+- `false` / `Promise<false>`: 接続不可（接続を切断）
+
+これにより、チャネルは接続確立前に早期可視性・サインインチェックを実行できます。
+
+#### Connection.connectChannel()の非同期化
+
+[Connection.connectChannel()メソッド](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/server/api/stream/Connection.ts)が非同期になり、初期化結果に基づいて接続を拒否できるようになりました:
+
+```typescript
+public async connectChannel(id: string, params: JsonObject | undefined, channel: string, pong = false) {
+ // ...
+ const valid = await ch.init(params ?? {});
+ if (typeof valid === 'boolean' && !valid) {
+ // 初期化処理の結果、接続拒否されたので切断
+ this.disconnectChannel(id);
+ return;
+ }
+ // ...
+}
+```
+
+#### 早期可視性フィルタリング
+
+[Connection.onNoteStreamMessage()](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/server/api/stream/Connection.ts)は、ストリーミングイベントのメタデータを使用して早期フィルタリングを実行します:
+
+```typescript
+private async onNoteStreamMessage(data: GlobalEvents['note']['payload']) {
+ // 自分自身ではないかつ
+ if (data.body.userId !== this.user?.id) {
+ // 公開範囲が指名で自分が含まれてない
+ if (data.body.visibility === 'specified' &&
+ (this.user == null || (!data.body.visibleUserIds.includes(this.user.id) &&
+ !(data.body.mentions?.includes(this.user.id) ?? false)))) {
+ return;
+ }
+
+ // 公開範囲がフォロワーで自分がフォロワーでない
+ if (data.body.visibility === 'followers' &&
+ (this.user == null || (!Object.hasOwn(this.following, data.body.userId) &&
+ data.body.replyUserId !== this.user.id &&
+ !(data.body.mentions?.includes(this.user.id) ?? false)))) {
+ return;
+ }
+ }
+ // ...
+}
+```
+
+これにより、各チャネルに配信する前に、接続レベルで不適切なNoteがフィルタリングされます。
+
+#### チャネル管理の最適化
+
+チャネルストレージが配列から`Map<string, Channel>`に変更され、接続管理のパフォーマンスが向上しました:
+
+```typescript
+private channels: Map<string, Channel> = new Map();
+```
+
+これにより、O(n)の線形検索からO(1)の定数時間検索に改善されます。
+
+### 5.4 タイムラインチャネルでの可視性適用
+
+各タイムラインチャネル（[home-timeline](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/server/api/stream/channels/home-timeline.ts)、[local-timeline](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/server/api/stream/channels/local-timeline.ts)など）は、`NoteStreamingHidingService`を統合しています:
+
+```typescript
+const { shouldSkip } = await this.noteStreamingHidingService.processHiding(note, user.id);
+if (shouldSkip) return;
+```
+
+#### Channel.isNoteVisibleForMe()ヘルパー
+
+[Channelベースクラス](https://github.com/MisskeyIO/misskey/blob/develop/packages/backend/src/server/api/stream/channel.ts)は、`QueryService.generateVisibilityQuery`と同期された可視性チェックヘルパーを提供します:
+
+```typescript
+protected isNoteVisibleForMe(note: Packed<'Note'>): boolean {
+ const meId = this.connection.user?.id ?? null;
+
+ if (note.visibility === 'specified') {
+ if (meId == null) {
+ return false;
+ } else if (meId === note.userId) {
+ return true;
+ } else {
+ return (note.visibleUserIds?.some(id => meId === id) ?? false)
+ || (note.mentions?.some(id => meId === id) ?? false);
+ }
+ }
+
+ if (note.visibility === 'followers') {
+ if (meId == null) {
+ return false;
+ } else if (meId === note.userId) {
+ return true;
+ } else if (note.reply && (meId === note.reply.userId)) {
+ return true;
+ } else if (note.mentions && note.mentions.some(id => meId === id)) {
+ return true;
+ } else {
+ return Object.hasOwn(this.following, note.userId);
+ }
+ }
+
+ return true;
+}
+```
+
+このヘルパーにより、タイムラインチャネル全体で一貫した可視性チェックが保証されます。
+
+## 6. これらのパターンがUI応答性とロード安定性を向上させる仕組み
+
+### 6.1 UI応答性の利点
**Async/Awaitによるメインスレッドブロッキング防止:**
- すべての取得操作はPromiseで即座に返される
@@ -350,7 +657,7 @@
- ユーザーがキューに入った更新をいつロードするかを制御
- 読書体験が安定し予測可能になる
-### 5.2 ロード安定性の利点
+### 6.2 ロード安定性の利点
**個別のローディング状態:**
- 初期ロードはコンテンツフラッシュなしでスピナーを表示
@@ -372,7 +679,7 @@
- Note作成における競合状態を排除
- 可用性を犠牲にすることなくデータ一貫性を保証
-### 5.3 パフォーマンス特性
+### 6.3 パフォーマンス特性
**最小化されたNoteストリーミング:**
- WebSocketは必須データ（ID）のみを配信
@@ -390,6 +697,24 @@
- 一時的なネットワーク問題からの高速リカバリ
- 初期リトライの総待機時間が短く、より良いユーザー体験
+### 6.4 ストリーミング可視性制御の利点
+
+**早期フィルタリング:**
+- 接続レベルでの可視性チェックにより、不適切なNoteがチャネルに到達する前にブロック
+- 追加のデータベースクエリなしで、メタデータベースのフィルタリングが実行される
+- チャネル処理オーバーヘッドを削減
+
+**多層リノート処理:**
+- ネストされたリノート構造の各レイヤーで個別の可視性判定
+- 純粋リノートは可視性違反時に完全にスキップ
+- 引用リノートは違反部分のみを隠して配信を継続
+- ユーザー体験と情報セキュリティのバランスを実現
+
+**パフォーマンス最適化:**
+- `userFollowingsCache`の使用によりフォロワーチェックが高速化
+- チャネル管理が`Map`ベースになりO(1)の検索時間
+- モジュール化されたヘルパーメソッドによりコードの再利用性が向上
+
## まとめ
MisskeyのNote取得アーキテクチャは以下を組み合わせています:
@@ -399,9 +724,11 @@
3. **フィボナッチバックオフリトライ** 指数戦略よりも緩やかな進行
4. **非同期パターン** PromiseベースのAPI、遅延読み込み、キュー管理を含む
5. **UI最適化** IntersectionObserverとCSS content-visibilityを活用
+6. **ストリーミング可視性制御** NoteStreamingHidingServiceによる多層リノート処理と早期フィルタリング
これらのパターンが連携して以下を保証します:
- **応答性**: 取得中にUIがブロックされない
- **安定性**: ローディング状態とキューが破壊的な更新を防止
- **効率性**: 遅延読み込みとバッチ処理が不要な作業を最小化
- **信頼性**: リトライロジックと重複防止が障害を適切に処理
+- **セキュリティ**: 可視性ルールの厳格な適用により情報漏洩を防止How did I do? Any feedback? |
Greptile SummaryこのPRは2つの主要なセキュリティ修正とストリーミング配信パイプラインのリファクタリングを行っています。①インポート系エンドポイント（import-antennas/blocking/following/muting/user-lists）で 主な変更点：
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant C as クライアント
participant Conn as Connection
participant Ch as Channel (各タイムライン)
participant NHide as NoteStreamingHidingService
participant NEntSvc as NoteEntityService
C->>Conn: WebSocket 接続
Conn->>Ch: connectChannel(id, params, channel)
Ch->>Ch: init() - 認証・オーナーシップ確認
alt 接続不可 (false 返却)
Ch-->>Conn: false
Conn->>Ch: disconnectChannel(id)
else 接続可
Ch-->>Conn: true / void
Note over Conn,Ch: イベント購読開始
Note over Ch: notesStream イベント発火
Ch->>Ch: isNoteVisibleForMe(note)
alt 不可視
Ch-->>C: 配信なし
else 可視
Ch->>Ch: isNoteMutedOrBlocked(note)
alt ミュート/ブロック
Ch-->>C: 配信なし
else 通過
Ch->>NHide: processHiding(note, meId)
NHide->>NEntSvc: shouldHideNote(note, meId) ×最大3階層
NEntSvc-->>NHide: boolean
alt shouldSkip = true
NHide-->>Ch: shouldSkip: true
Ch-->>C: 配信なし
else shouldSkip = false
NHide->>NEntSvc: hideNote(note) [該当層のみ]
NHide-->>Ch: shouldSkip: false
Ch->>C: 隠しフィールド適用済みノートを配信
end
end
end
end
Note over Conn: noteStream (個別ノート更新)
Conn->>Conn: onNoteStreamMessage(data)
alt visibility=specified かつ未指定ユーザー
Conn-->>C: 配信なし
else visibility=followers かつ非フォロワー
Conn-->>C: 配信なし
else 通過
Conn->>C: noteUpdated イベント送信
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac0ecb5170
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/backend/src/server/api/stream/channels/user-list.ts (1)
44-58:⚠️ Potential issue | 🟠 Major
requireCredentialと実際の使用に不整合があります。
UserListChannelではrequireCredential = falseですが、Line 55でthis.user!.idを使用しています。未認証ユーザーが接続した場合、this.userはundefinedとなりクラッシュします。リスト所有者のみが接続できるべきであれば、
requireCredential = trueに変更すべきです。🐛 修正案
class UserListChannel extends Channel { public readonly chName = 'userList'; public static readonly shouldShare = false; - public static readonly requireCredential = false as const; + public static readonly requireCredential = true as const;または、initで早期リターンを追加：
`@bindThis` public async init(params: JsonObject): Promise<boolean> { if (typeof params.listId !== 'string') return false; + if (!this.user) return false; this.listId = params.listId;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/user-list.ts` around lines 44 - 58, The channel sets requireCredential = false but init() uses this.user!.id which will crash for unauthenticated users; either make the channel require authentication by setting requireCredential = true on the UserListChannel class, or add a guard in init() that checks this.user exists and returns false if not (before calling userListsRepository.exists), and ensure any subsequent logic that assumes a user is behind that check; update the class field or add the early return in init() near the this.user!.id usage.packages/backend/src/server/api/stream/channels/hashtag.ts (1)
61-67:⚠️ Potential issue | 🟠 Major未認証ユーザーに対するnull参照の可能性があります。
HashtagChannelはrequireCredential = falseであるため、未認証ユーザーが接続可能です。Line 66でthis.user!.idにアクセスしていますが、this.userがnullの場合にクラッシュします。
visibility === 'specified'のノートは認証が必要なので、未認証ユーザーには表示されるべきではありませんが、早期リターンの前にnullチェックが必要です。🐛 修正案
if (note.reply) { const reply = note.reply; // 自分のフォローしていないユーザーの visibility: followers な投稿への返信は弾く - if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId)) return; + if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId) && reply.userId !== this.user?.id) return; // 自分の見ることができないユーザーの visibility: specified な投稿への返信は弾く - if (reply.visibility === 'specified' && !reply.visibleUserIds!.includes(this.user!.id)) return; + if (reply.visibility === 'specified' && (!this.user || !reply.visibleUserIds!.includes(this.user.id))) return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/hashtag.ts` around lines 61 - 67, In the reply-visibility check inside HashtagChannel (the block handling note.reply), avoid dereferencing this.user when unauthenticated: before checking reply.visibility === 'specified' and calling reply.visibleUserIds!.includes(this.user!.id), add a null/undefined guard for this.user (e.g., return early if this.user is falsy) or change the condition to require this.user to exist (e.g., if (reply.visibility === 'specified' && (!this.user || !reply.visibleUserIds!.includes(this.user.id)) ) return;), so no null reference occurs for unauthenticated connections.packages/backend/src/server/api/stream/Connection.ts (1)
206-221:⚠️ Potential issue | 🔴 Critical未認証ユーザーによるnull参照エラーを修正してください。
onNoteStreamMessageメソッド内でthis.user!.idにアクセスしていますが、未認証ユーザー（this.user === undefined）でもonSubscribeNoteを通じてリスナーが登録される可能性があります。未認証接続からノート購読要求が来た場合、イベント受信時にTypeErrorが発生します。修正案
`@bindThis` private async onNoteStreamMessage(data: GlobalEvents['note']['payload']) { + if (!this.user) return; + if (data.body.visibility === 'specified' && !data.body.visibleUserIds.includes(this.user!.id)) { return; } - if (data.body.visibility === 'followers' && !Object.hasOwn(this.following, data.body.userId)) { + if (data.body.visibility === 'followers' && data.body.userId !== this.user.id && !Object.hasOwn(this.following, data.body.userId)) { return; } this.sendMessageToWs('noteUpdated', {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/Connection.ts` around lines 206 - 221, The handler onNoteStreamMessage currently uses the non-null assertion this.user!.id causing a TypeError for unauthenticated connections; update onNoteStreamMessage to guard access to this.user before checking visibility (e.g., if data.body.visibility === 'specified' && (!this.user || !data.body.visibleUserIds.includes(this.user.id)) return;), removing the non-null assertion, and ensure similar safe checks remain for follower visibility logic (use this.user existence or appropriate checks in the followers branch) so no property is accessed on undefined.
🧹 Nitpick comments (2)
packages/backend/src/server/api/stream/channels/chat-room.ts (1)
22-31:@Injectデコレータは手動でインスタンス化されるクラスでは機能しません。
ChatRoomChannelはChatRoomChannelService.create()内でnewキーワードにより手動でインスタンス化されています。NestJSの@InjectデコレータはDIコンテナによって管理されるクラスでのみ機能するため、Line 23の@Inject(DI.chatRoomsRepository)は効果がありません。現在の実装では
create()メソッドで明示的にthis.chatRoomsRepositoryを渡しているため動作しますが、デコレータを削除してコードの意図を明確にすることを推奨します。♻️ 修正案
constructor( - `@Inject`(DI.chatRoomsRepository) private chatRoomsRepository: ChatRoomsRepository, private chatService: ChatService, id: string, connection: Channel['connection'], ) { super(id, connection, null); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/chat-room.ts` around lines 22 - 31, ChatRoomChannel's constructor is using `@Inject`(DI.chatRoomsRepository) which has no effect because ChatRoomChannel is manually instantiated in ChatRoomChannelService.create(); remove the `@Inject` decorator from the constructor parameter, make the parameter a plain typed parameter (e.g., chatRoomsRepository: ChatRoomsRepository) and keep the existing code path in ChatRoomChannelService.create() that explicitly passes the repository into new ChatRoomChannel(...) so the dependency is provided manually rather than via Nest DI.packages/backend/src/server/api/stream/channels/antenna.ts (1)
26-38:@Injectデコレータの使用に関する注意点。
chat-room.tsと同様、AntennaChannelは手動でインスタンス化されるため、Line 27-28の@Injectデコレータは機能しません。create()メソッドで明示的にリポジトリを渡しているため動作はしますが、デコレータの削除を推奨します。♻️ 修正案
constructor( - `@Inject`(DI.antennasRepository) private antennasRepository: AntennasRepository, private roleService: RoleService, private noteEntityService: NoteEntityService, private noteStreamingHidingService: NoteStreamingHidingService, id: string, connection: Channel['connection'], ) { super(id, connection, null); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/antenna.ts` around lines 26 - 38, Remove the non-functional `@Inject`(DI.antennasRepository) decorator on the constructor parameter in AntennaChannel (the antennasRepository parameter) because AntennaChannel is manually instantiated and repositories are passed explicitly via the create() method; update the constructor signature to just accept private antennasRepository: AntennasRepository (and remove the `@Inject` import if it becomes unused) so dependency wiring remains explicit and consistent with chat-room.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/server/api/stream/channel.ts`:
- Around line 77-112: In isNoteVisibleForMe (method isNoteVisibleForMe) the
'specified' branch only checks note.visibleUserIds and misses note.mentions,
causing divergence from QueryService.generateVisibilityQuery; update the
'specified' branch to also allow visibility if the current user
(this.connection.user?.id) is mentioned (note.mentions contains meId) or is the
author (note.userId), mirroring the same conditions used in
QueryService.generateVisibilityQuery and other callers (e.g., channels/main.ts).
- Around line 156-163: The init() return semantics are inconsistent: change the
bare early-returns in the channel init implementations so they explicitly return
false when validation fails (so Connection.ts’s check with const valid = await
ch.init(...) and typeof valid === 'boolean' && !valid will disconnect); update
the init methods in the channel implementations named channel.ts (the init in
that class), global-timeline.ts, local-timeline.ts, and role-timeline.ts to
return false instead of bare return when params or policy checks fail, leaving
other successful/unchecked paths as void/true as before and keeping
disconnectChannel usage unchanged.
---
Outside diff comments:
In `@packages/backend/src/server/api/stream/channels/hashtag.ts`:
- Around line 61-67: In the reply-visibility check inside HashtagChannel (the
block handling note.reply), avoid dereferencing this.user when unauthenticated:
before checking reply.visibility === 'specified' and calling
reply.visibleUserIds!.includes(this.user!.id), add a null/undefined guard for
this.user (e.g., return early if this.user is falsy) or change the condition to
require this.user to exist (e.g., if (reply.visibility === 'specified' &&
(!this.user || !reply.visibleUserIds!.includes(this.user.id)) ) return;), so no
null reference occurs for unauthenticated connections.
In `@packages/backend/src/server/api/stream/channels/user-list.ts`:
- Around line 44-58: The channel sets requireCredential = false but init() uses
this.user!.id which will crash for unauthenticated users; either make the
channel require authentication by setting requireCredential = true on the
UserListChannel class, or add a guard in init() that checks this.user exists and
returns false if not (before calling userListsRepository.exists), and ensure any
subsequent logic that assumes a user is behind that check; update the class
field or add the early return in init() near the this.user!.id usage.
In `@packages/backend/src/server/api/stream/Connection.ts`:
- Around line 206-221: The handler onNoteStreamMessage currently uses the
non-null assertion this.user!.id causing a TypeError for unauthenticated
connections; update onNoteStreamMessage to guard access to this.user before
checking visibility (e.g., if data.body.visibility === 'specified' &&
(!this.user || !data.body.visibleUserIds.includes(this.user.id)) return;),
removing the non-null assertion, and ensure similar safe checks remain for
follower visibility logic (use this.user existence or appropriate checks in the
followers branch) so no property is accessed on undefined.
---
Nitpick comments:
In `@packages/backend/src/server/api/stream/channels/antenna.ts`:
- Around line 26-38: Remove the non-functional `@Inject`(DI.antennasRepository)
decorator on the constructor parameter in AntennaChannel (the antennasRepository
parameter) because AntennaChannel is manually instantiated and repositories are
passed explicitly via the create() method; update the constructor signature to
just accept private antennasRepository: AntennasRepository (and remove the
`@Inject` import if it becomes unused) so dependency wiring remains explicit and
consistent with chat-room.ts.
In `@packages/backend/src/server/api/stream/channels/chat-room.ts`:
- Around line 22-31: ChatRoomChannel's constructor is using
`@Inject`(DI.chatRoomsRepository) which has no effect because ChatRoomChannel is
manually instantiated in ChatRoomChannelService.create(); remove the `@Inject`
decorator from the constructor parameter, make the parameter a plain typed
parameter (e.g., chatRoomsRepository: ChatRoomsRepository) and keep the existing
code path in ChatRoomChannelService.create() that explicitly passes the
repository into new ChatRoomChannel(...) so the dependency is provided manually
rather than via Nest DI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab038437-5125-4114-821d-9627dfb5423a
📒 Files selected for processing (31)
locales/index.d.tspackages/backend/src/core/GlobalEventService.tspackages/backend/src/core/NoteDeleteService.tspackages/backend/src/core/PollService.tspackages/backend/src/core/QueryService.tspackages/backend/src/core/ReactionService.tspackages/backend/src/core/entities/NoteEntityService.tspackages/backend/src/server/ActivityPubServerService.tspackages/backend/src/server/ServerModule.tspackages/backend/src/server/api/endpoints/i/import-antennas.tspackages/backend/src/server/api/endpoints/i/import-blocking.tspackages/backend/src/server/api/endpoints/i/import-following.tspackages/backend/src/server/api/endpoints/i/import-muting.tspackages/backend/src/server/api/endpoints/i/import-user-lists.tspackages/backend/src/server/api/endpoints/notes/polls/vote.tspackages/backend/src/server/api/stream/Connection.tspackages/backend/src/server/api/stream/NoteStreamingHidingService.tspackages/backend/src/server/api/stream/channel.tspackages/backend/src/server/api/stream/channels/antenna.tspackages/backend/src/server/api/stream/channels/channel.tspackages/backend/src/server/api/stream/channels/chat-room.tspackages/backend/src/server/api/stream/channels/chat-user.tspackages/backend/src/server/api/stream/channels/global-timeline.tspackages/backend/src/server/api/stream/channels/hashtag.tspackages/backend/src/server/api/stream/channels/home-timeline.tspackages/backend/src/server/api/stream/channels/hybrid-timeline.tspackages/backend/src/server/api/stream/channels/local-timeline.tspackages/backend/src/server/api/stream/channels/main.tspackages/backend/src/server/api/stream/channels/role-timeline.tspackages/backend/src/server/api/stream/channels/user-list.tspackages/backend/src/types.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/server/api/stream/Connection.ts`:
- Around line 299-305: The bug is that this.channels.set(id, ch) happens before
awaiting ch.init(params) so a concurrent reconnect/disconnect can replace the
map entry and the post-init logic (disconnectChannel(id) and the "connected"
message) will act on the new channel; to fix, after awaiting ch.init(params)
(and in the catch block) verify that this.channels.get(id) === ch before calling
this.disconnectChannel(id), sending the connected message, or performing any
cleanup, and if init throws ensure you only remove/cleanup the channel when the
map still references this same ch to avoid affecting a newer connection and to
avoid leaving unhandled promises.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0dd669c2-ba5d-4ddb-9cbf-8f0679a96d54
📒 Files selected for processing (1)
packages/backend/src/server/api/stream/Connection.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/backend/src/server/api/stream/channels/channel.ts (1)
63-79:⚠️ Potential issue | 🟠 Major
cacheNoteの条件がisNoteVisibleForMe()とまだ一致していません。Line 63 で mention 経由の
specifiedノートも通るようになった一方、Line 78 のnote.visibleUserIds?.includes(...) ?? note.mentions?.includes(...)は左辺がfalseでも右辺を見ません。visibleUserIdsはあるが自分はmentionsでだけ見えるノートは配信されてもキャッシュから漏れるので、後段の更新系判定がまたずれます。修正案
- if (this.user && (note.visibleUserIds?.includes(this.user.id) ?? note.mentions?.includes(this.user.id))) { + if (this.user && ( + (note.visibleUserIds?.includes(this.user.id) ?? false) + || (note.mentions?.includes(this.user.id) ?? false) + )) { this.connection.cacheNote(note); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/channel.ts` around lines 63 - 79, The cache condition after processing (involving cacheNote) does not match isNoteVisibleForMe() because it uses the nullish coalescing operator (??) which skips the right-hand check when visibleUserIds exists but does not include the user; change the condition used before calling this.connection.cacheNote(note) to explicitly check both collections (e.g. use a logical OR between note.visibleUserIds?.includes(this.user.id) and note.mentions?.includes(this.user.id), with null-safe guards) so notes visible only via mentions still get cached; update the check near cacheNote and ensure it mirrors the visibility logic in isNoteVisibleForMe.
♻️ Duplicate comments (1)
packages/backend/src/server/api/stream/Connection.ts (1)
299-305:⚠️ Potential issue | 🔴 Critical既出ですが、
await ch.init()中の再接続競合が未解消です。
Mapへ登録してからawaitしているので、その間に同じidの再接続/切断が入ると、ここでのdisconnectChannel(id)と後続のconnected送信が新しいチャネルに作用します。ch.init()の例外も未処理のままなので、await後とcatchの両方でthis.channels.get(id) === chを確認してから cleanup/応答してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/Connection.ts` around lines 299 - 305, The bug is that we register the channel with this.channels.set(ch.id, ch) before awaiting ch.init(params) so concurrent reconnects/disconnects can cause cleanup or the subsequent "connected" send to affect a different channel; also ch.init() exceptions are not handled. Fix by moving the registration until after a successful await ch.init(params) (or, if you must set early, then after each await/catch check that this.channels.get(id) === ch before calling disconnectChannel(id), sending the "connected" response, or performing any cleanup), and wrap ch.init(params) in try/catch where the catch also verifies this.channels.get(id) === ch before doing any teardown or response.
🧹 Nitpick comments (1)
packages/backend/src/server/api/stream/Connection.ts (1)
208-219:noteUpdatedの可視性判定も共通ロジックへ寄せてください。ここだけ
specified/followersを手書きすると、他チャネル側の隠蔽・可視性判定と将来ずれやすいです。noteUpdatedも同じサービス/ヘルパー経由に揃えておく方が保守しやすく、安全です。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/Connection.ts` around lines 208 - 219, The visibility checks in Connection.ts for noteUpdated (the block inspecting data.body.visibility, visibleUserIds, mentions, replyUserId and using this.user/this.following) must be delegated to the shared visibility logic used by other channels; refactor this conditional to call the common helper/service (e.g., a shared isNoteVisibleTo or VisibilityService method) that accepts the note payload (data.body) and the current user context (this.user, this.following) and returns a boolean, then return early based on that result instead of duplicating the 'specified'/'followers' logic inline in noteUpdated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/server/api/stream/channel.ts`:
- Around line 77-113: The per-channel duplicated visibility logic should be
removed and unified: replace the inline visibility checks in the channel
implementations with calls to the existing helper isNoteVisibleForMe, invoking
it for the main note and also for any nested reply (e.g., call
isNoteVisibleForMe(note) and if note.reply exists call
isNoteVisibleForMe(note.reply)); ensure you remove the separate
mentions/followers/reply special-case code in those channel methods so both root
and reply-side visibility use the same isNoteVisibleForMe helper.
---
Outside diff comments:
In `@packages/backend/src/server/api/stream/channels/channel.ts`:
- Around line 63-79: The cache condition after processing (involving cacheNote)
does not match isNoteVisibleForMe() because it uses the nullish coalescing
operator (??) which skips the right-hand check when visibleUserIds exists but
does not include the user; change the condition used before calling
this.connection.cacheNote(note) to explicitly check both collections (e.g. use a
logical OR between note.visibleUserIds?.includes(this.user.id) and
note.mentions?.includes(this.user.id), with null-safe guards) so notes visible
only via mentions still get cached; update the check near cacheNote and ensure
it mirrors the visibility logic in isNoteVisibleForMe.
---
Duplicate comments:
In `@packages/backend/src/server/api/stream/Connection.ts`:
- Around line 299-305: The bug is that we register the channel with
this.channels.set(ch.id, ch) before awaiting ch.init(params) so concurrent
reconnects/disconnects can cause cleanup or the subsequent "connected" send to
affect a different channel; also ch.init() exceptions are not handled. Fix by
moving the registration until after a successful await ch.init(params) (or, if
you must set early, then after each await/catch check that this.channels.get(id)
=== ch before calling disconnectChannel(id), sending the "connected" response,
or performing any cleanup), and wrap ch.init(params) in try/catch where the
catch also verifies this.channels.get(id) === ch before doing any teardown or
response.
---
Nitpick comments:
In `@packages/backend/src/server/api/stream/Connection.ts`:
- Around line 208-219: The visibility checks in Connection.ts for noteUpdated
(the block inspecting data.body.visibility, visibleUserIds, mentions,
replyUserId and using this.user/this.following) must be delegated to the shared
visibility logic used by other channels; refactor this conditional to call the
common helper/service (e.g., a shared isNoteVisibleTo or VisibilityService
method) that accepts the note payload (data.body) and the current user context
(this.user, this.following) and returns a boolean, then return early based on
that result instead of duplicating the 'specified'/'followers' logic inline in
noteUpdated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b535c465-36d7-45e0-9ab6-053f0689bf76
📒 Files selected for processing (7)
packages/backend/src/core/GlobalEventService.tspackages/backend/src/server/api/stream/Connection.tspackages/backend/src/server/api/stream/channel.tspackages/backend/src/server/api/stream/channels/channel.tspackages/backend/src/server/api/stream/channels/global-timeline.tspackages/backend/src/server/api/stream/channels/local-timeline.tspackages/backend/src/server/api/stream/channels/role-timeline.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/backend/src/server/api/stream/channels/global-timeline.ts (1)
71-75:⚠️ Potential issue | 🟠 Major未認証ユーザーに対する潜在的なランタイムエラー。
GlobalTimelineChannelはrequireCredential = falseですが、Line 73でthis.user!.idと非nullアサーションを使用しています。未認証ユーザーがwithRepliesフォロー設定なしでリプライを含むノートを受信した場合、このコードパスに到達し、TypeErrorが発生する可能性があります。🐛 提案される修正
} else { // 「チャンネル接続主への返信」でもなければ、「チャンネル接続主が行った返信」でもなければ、「投稿者の投稿者自身への返信」でもない場合 - if (reply.userId !== this.user!.id && note.userId !== this.user!.id && reply.userId !== note.userId) return; + if (this.user == null) return; + if (reply.userId !== this.user.id && note.userId !== this.user.id && reply.userId !== note.userId) return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/global-timeline.ts` around lines 71 - 75, The code in GlobalTimelineChannel uses a non-null assertion this.user!.id while requireCredential = false, which can throw for unauthenticated users; change the conditional that checks reply.userId and note.userId to first handle the unauthenticated case: allow the note when reply.userId === note.userId (poster replying to self) OR, if this.user exists, allow when reply.userId === this.user.id or note.userId === this.user.id; otherwise return. Update the condition around reply.userId/note.userId/this.user (the block containing reply.userId !== this.user!.id && note.userId !== this.user!.id && reply.userId !== note.userId) to explicitly guard access to this.user and avoid using this.user!.packages/backend/src/server/api/stream/channels/user-list.ts (1)
71-86:⚠️ Potential issue | 🔴 Critical
withRepliesフィールドがselectに含まれていないため、常にundefinedになります73行目の
select: ['userId']ではwithRepliesフィールドが取得されていないため、83行目で割り当てられるmembership.withRepliesは常にundefinedになります。その結果、106行目のthis.membershipsMap[note.userId]?.withRepliesチェックが期待通りに機能しません。selectに
'withReplies'を追加してください:修正例
const memberships = await this.userListMembershipsRepository.find({ where: { userListId: this.listId, }, - select: ['userId'], + select: ['userId', 'withReplies'], });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/user-list.ts` around lines 71 - 86, The query in updateListUsers uses userListMembershipsRepository.find with select: ['userId'] so membership.withReplies is never returned; update the find call to include 'withReplies' in the select array (so membership.withReplies is populated), ensure the membershipsMap assignment uses the returned withReplies, and adjust any typing for membershipsMap or MiUserListMembership selection if necessary to reflect that withReplies is now fetched.packages/backend/src/server/api/stream/channels/local-timeline.ts (1)
70-77:⚠️ Potential issue | 🔴 Critical匿名接続で返信ノートが来ると
this.user!.idで落ちます。
requireCredential = falseなのでthis.userはnullになり得ますが、Line 76 で非 null 断言しています。withRepliesが無効な匿名セッションに返信が流れるとここで例外になるので、匿名時は早期 return するかthis.userの存在確認を入れてください。修正案
if (note.reply) { const reply = note.reply; if ((this.following[note.userId]?.withReplies ?? false) || this.withReplies) { if (!this.isNoteVisibleForMe(reply)) return; - } else { + } else if (this.user == null) { + return; + } else { // 「チャンネル接続主への返信」でもなければ、「チャンネル接続主が行った返信」でもなければ、「投稿者の投稿者自身への返信」でもない場合 - if (reply.userId !== this.user!.id && note.userId !== this.user!.id && reply.userId !== note.userId) return; + if (reply.userId !== this.user.id && note.userId !== this.user.id && reply.userId !== note.userId) return; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/local-timeline.ts` around lines 70 - 77, The code assumes this.user is non-null when checking reply visibility (uses this.user!.id) which crashes for anonymous sessions; update the logic in the reply-handling block of local-timeline.ts (around the check that reads this.user!.id) to first handle anonymous sessions by either early-returning when requireCredential is false and withReplies is not set for the follower, or by guarding access to this.user (e.g., check if this.user exists before using this.user.id) so you never use a non-null assertion; keep the existing branch behavior when this.user is present and use isNoteVisibleForMe, withReplies, and reply.userId/note.userId comparisons only after confirming this.user.
🧹 Nitpick comments (3)
packages/backend/src/server/api/stream/channels/home-timeline.ts (1)
88-89: processHidingの統合は正しいですが、nullish coalescingは不要です。
HomeTimelineChannelはrequireCredential = trueなので、this.userは常に存在します。this.user?.id ?? nullはthis.user.idに簡略化できます。♻️ 提案される修正
- const { shouldSkip } = await this.noteStreamingHidingService.processHiding(note, this.user?.id ?? null); + const { shouldSkip } = await this.noteStreamingHidingService.processHiding(note, this.user.id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/home-timeline.ts` around lines 88 - 89, The code calls noteStreamingHidingService.processHiding with this.user?.id ?? null which is unnecessary because HomeTimelineChannel has requireCredential = true and this.user is always present; replace the nullable expression with this.user.id when invoking processHiding (i.e., update the call in HomeTimelineChannel to use this.user.id) so processHiding receives a definite user id.packages/backend/src/server/api/stream/channels/hybrid-timeline.ts (1)
102-109:reactionsプロパティへの防御的なアクセスを推奨します。104行目で
note.renote.reactionsがundefinedの場合、Object.keys(undefined)がTypeErrorをスローする可能性があります。防御的コーディングとしてオプショナルチェーンを追加することを推奨します。♻️ 防御的コーディングの提案
if (this.user) { if (isRenotePacked(note) && !isQuotePacked(note)) { - if (note.renote && Object.keys(note.renote.reactions).length > 0) { + if (note.renote?.reactions && Object.keys(note.renote.reactions).length > 0) { const myRenoteReaction = await this.noteEntityService.populateMyReaction(note.renote, this.user.id); note.renote.myReaction = myRenoteReaction; } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/hybrid-timeline.ts` around lines 102 - 109, The code assumes note.renote.reactions exists and calls Object.keys(note.renote.reactions), which can throw if reactions is undefined; update the check around the renote reaction handling in hybrid-timeline so it defensively verifies reactions (e.g., use optional chaining or an explicit null/undefined check on note.renote.reactions) before calling Object.keys, then proceed to call this.noteEntityService.populateMyReaction(note.renote, this.user.id) and assign note.renote.myReaction only when reactions are present; reference the symbols note.renote.reactions, note.renote.myReaction, and populateMyReaction to locate the code to change.packages/backend/src/server/api/stream/channels/user-list.ts (1)
44-68: initメソッドのboolean戻り値への変更は良好です初期化の成功/失敗を明示的に返すパターンにより、呼び出し側でエラーハンドリングがしやすくなっています。
注意点: 55行目で
this.user!.idを使用していますが、21行目でrequireCredential = falseと定義されています。これは既存コードですが、this.userがnullの場合にランタイムエラーが発生する可能性があります。所有権チェックが必須であれば、requireCredentialをtrueに変更するか、nullチェックの追加を検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/user-list.ts` around lines 44 - 68, The init method uses a non-null assertion this.user!.id while the stream class sets requireCredential = false, risking a runtime crash; either set requireCredential = true on the class (so this.user is guaranteed) or add an explicit null-check at the start of init (e.g., if (!this.user) return false) before calling userListsRepository.exists and before subscribing/using this.user, ensuring safe handling and returning false when credentials are missing; update any related logic that assumes ownership (userListsRepository.exists, subscription setup, updateListUsers) to only run when this.user is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/server/api/stream/channels/antenna.ts`:
- Around line 27-28: Remove the non-functional `@Inject` decorator from the
AntennaChannel constructor property and keep only the plain parameter for
antennasRepository; specifically, update the AntennaChannel class to stop using
`@Inject` on the antennasRepository member (and any other `@Inject` usages in that
class), relying on the existing create() method which already manually passes
the AntennasRepository dependency to the constructor.
In `@packages/backend/src/server/api/stream/channels/hybrid-timeline.ts`:
- Around line 99-100: The code mixes non-null-assertion this.user!.id and
defensive this.user?.id ?? null; update the usage at the earlier occurrence
(where this.user!.id is passed) to use the defensive pattern this.user?.id ??
null so both calls to noteStreamingHidingService.processHiding use the same
null-tolerant style; locate the earlier call that currently passes this.user!.id
and replace it with this.user?.id ?? null.
---
Outside diff comments:
In `@packages/backend/src/server/api/stream/channels/global-timeline.ts`:
- Around line 71-75: The code in GlobalTimelineChannel uses a non-null assertion
this.user!.id while requireCredential = false, which can throw for
unauthenticated users; change the conditional that checks reply.userId and
note.userId to first handle the unauthenticated case: allow the note when
reply.userId === note.userId (poster replying to self) OR, if this.user exists,
allow when reply.userId === this.user.id or note.userId === this.user.id;
otherwise return. Update the condition around reply.userId/note.userId/this.user
(the block containing reply.userId !== this.user!.id && note.userId !==
this.user!.id && reply.userId !== note.userId) to explicitly guard access to
this.user and avoid using this.user!.
In `@packages/backend/src/server/api/stream/channels/local-timeline.ts`:
- Around line 70-77: The code assumes this.user is non-null when checking reply
visibility (uses this.user!.id) which crashes for anonymous sessions; update the
logic in the reply-handling block of local-timeline.ts (around the check that
reads this.user!.id) to first handle anonymous sessions by either
early-returning when requireCredential is false and withReplies is not set for
the follower, or by guarding access to this.user (e.g., check if this.user
exists before using this.user.id) so you never use a non-null assertion; keep
the existing branch behavior when this.user is present and use
isNoteVisibleForMe, withReplies, and reply.userId/note.userId comparisons only
after confirming this.user.
In `@packages/backend/src/server/api/stream/channels/user-list.ts`:
- Around line 71-86: The query in updateListUsers uses
userListMembershipsRepository.find with select: ['userId'] so
membership.withReplies is never returned; update the find call to include
'withReplies' in the select array (so membership.withReplies is populated),
ensure the membershipsMap assignment uses the returned withReplies, and adjust
any typing for membershipsMap or MiUserListMembership selection if necessary to
reflect that withReplies is now fetched.
---
Nitpick comments:
In `@packages/backend/src/server/api/stream/channels/home-timeline.ts`:
- Around line 88-89: The code calls noteStreamingHidingService.processHiding
with this.user?.id ?? null which is unnecessary because HomeTimelineChannel has
requireCredential = true and this.user is always present; replace the nullable
expression with this.user.id when invoking processHiding (i.e., update the call
in HomeTimelineChannel to use this.user.id) so processHiding receives a definite
user id.
In `@packages/backend/src/server/api/stream/channels/hybrid-timeline.ts`:
- Around line 102-109: The code assumes note.renote.reactions exists and calls
Object.keys(note.renote.reactions), which can throw if reactions is undefined;
update the check around the renote reaction handling in hybrid-timeline so it
defensively verifies reactions (e.g., use optional chaining or an explicit
null/undefined check on note.renote.reactions) before calling Object.keys, then
proceed to call this.noteEntityService.populateMyReaction(note.renote,
this.user.id) and assign note.renote.myReaction only when reactions are present;
reference the symbols note.renote.reactions, note.renote.myReaction, and
populateMyReaction to locate the code to change.
In `@packages/backend/src/server/api/stream/channels/user-list.ts`:
- Around line 44-68: The init method uses a non-null assertion this.user!.id
while the stream class sets requireCredential = false, risking a runtime crash;
either set requireCredential = true on the class (so this.user is guaranteed) or
add an explicit null-check at the start of init (e.g., if (!this.user) return
false) before calling userListsRepository.exists and before subscribing/using
this.user, ensuring safe handling and returning false when credentials are
missing; update any related logic that assumes ownership
(userListsRepository.exists, subscription setup, updateListUsers) to only run
when this.user is present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1e94909f-5144-4469-8d05-d0312a52ba39
📒 Files selected for processing (9)
packages/backend/src/server/api/stream/channels/antenna.tspackages/backend/src/server/api/stream/channels/channel.tspackages/backend/src/server/api/stream/channels/global-timeline.tspackages/backend/src/server/api/stream/channels/hashtag.tspackages/backend/src/server/api/stream/channels/home-timeline.tspackages/backend/src/server/api/stream/channels/hybrid-timeline.tspackages/backend/src/server/api/stream/channels/local-timeline.tspackages/backend/src/server/api/stream/channels/role-timeline.tspackages/backend/src/server/api/stream/channels/user-list.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/backend/src/server/api/stream/channels/channel.ts
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/server/api/stream/channels/local-timeline.ts`:
- Around line 76-77: The early guard `if (!this.user) return;` blocks
unauthenticated viewers from receiving self-replies; change it so
unauthenticated connections only skip viewer-specific checks but still allow
replies where reply.userId === note.userId. Concretely, update the guard around
the reply filter in local-timeline.ts to not return for unauthenticated users
when the reply is a self-reply (check reply.userId vs note.userId), i.e., only
bail out for unauthenticated users when the reply is not from the same user as
the note; reference the existing symbols this.user, reply.userId, and
note.userId to locate and adjust the conditional.
In `@packages/backend/src/server/api/stream/channels/user-list.ts`:
- Line 63: init() currently subscribes to userListStream before awaiting
this.updateListUsers(), which can leave a subscription active if
updateListUsers() fails; change the flow so the initial sync is completed first
by calling and awaiting this.updateListUsers() before creating/subscribing
userListStream (reference: init, updateListUsers, userListStream), or if you
must subscribe first, wrap the await this.updateListUsers() in try/catch and
call userListStream.unsubscribe() (or the stream's teardown) in the catch to
ensure no half-initialized subscription remains.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2ee1a57-56af-4b30-99a1-b599aad62041
📒 Files selected for processing (6)
packages/backend/src/server/api/stream/channels/antenna.tspackages/backend/src/server/api/stream/channels/global-timeline.tspackages/backend/src/server/api/stream/channels/home-timeline.tspackages/backend/src/server/api/stream/channels/hybrid-timeline.tspackages/backend/src/server/api/stream/channels/local-timeline.tspackages/backend/src/server/api/stream/channels/user-list.ts
…t init Keep unauthenticated local timeline connections from dropping self-replies while still applying viewer-specific reply checks when needed. Also initialize user-list memberships before stream subscription to avoid half-initialized listeners on init failure.
- home-timeline.ts: processHidingメソッド呼び出しでthis.user?.id ?? nullを使用するよう変更 （user.idからthis.user?.id ?? nullへの移行でnull安全性を確保） - hybrid-timeline.ts: init()メソッドの戻り値型をPromise<void>からPromise<boolean>に変更 （早期リターンでreturn falseを明示的に返すことで、Connection.tsでの検証処理と整合）
Member
|
@greptileai 再レビュー |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
packages/backend/src/core/entities/NoteEntityService.ts (1)
148-197:shouldHideNoteのリファクタリングで関心の分離が適切に行われています。チェックロジックとミューテーションロジックを分離する良いリファクタリングです。ただし、静的解析ツールが指摘しているように、認知的複雑度が許容値を超えています（22 > 15）。
また、168-169行目および184行目で
.some(id => meId === id)を使用していますが、単純な等価チェックの場合は.includes(meId)の方が簡潔で可読性が高くなります。♻️ `.includes()` を使用した改善案
// visibility が specified かつ自分が指定されていなかったら非表示 if (packedNote.visibility === 'specified') { if (meId == null) { return true; } else { // 指定されているかどうか - const specified = (packedNote.visibleUserIds?.some(id => meId === id) ?? false) - || (packedNote.mentions?.some(id => meId === id) ?? false); + const specified = (packedNote.visibleUserIds?.includes(meId) ?? false) + || (packedNote.mentions?.includes(meId) ?? false); if (!specified) { return true; } } } // visibility が followers かつ自分が投稿者のフォロワーでなかったら非表示 if (packedNote.visibility === 'followers') { if (meId == null) { return true; } else if (packedNote.reply && (meId === packedNote.reply.userId)) { // 自分の投稿に対するリプライ return false; - } else if (packedNote.mentions && packedNote.mentions.some(id => meId === id)) { + } else if (packedNote.mentions?.includes(meId)) { // 自分へのメンション return false; } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/core/entities/NoteEntityService.ts` around lines 148 - 197, The shouldHideNote method has high cognitive complexity and uses .some(id => meId === id) where .includes(meId) is clearer; refactor by extracting small helper functions (e.g., isSpecifiedVisibleTo(packedNote, meId), isFollowerVisibleTo(packedNote, meId), and isSigninOrTimeHidden(packedNote, meId)) to encapsulate each visibility branch and return early, and replace occurrences of packedNote.visibleUserIds?.some(id => meId === id) and packedNote.mentions?.some(id => meId === id) with packedNote.visibleUserIds?.includes(meId) and packedNote.mentions?.includes(meId) respectively to reduce complexity and improve readability while keeping the public async shouldHideNote signature unchanged.packages/backend/src/server/api/stream/channels/chat-room.ts (2)
22-30:chatRoomsRepositoryにreadonly修飾子を追加してください。注入された依存関係は再代入されないため、
readonlyとしてマークすることで不変性を保証し、意図しない変更を防ぐことができます。♻️ 修正案
constructor( - private chatRoomsRepository: ChatRoomsRepository, + private readonly chatRoomsRepository: ChatRoomsRepository, private chatService: ChatService,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/chat-room.ts` around lines 22 - 30, コンストラクタの注入パラメータである chatRoomsRepository が再代入されないことを明示するため、constructor のパラメータ宣言で chatRoomsRepository に readonly 修飾子を追加してください（対象シンボル: chatRoomsRepository, ChatRoomsRepository, constructor in chat-room.ts）。他の注入済みパラメータ（例: chatService）と同様に readonly として宣言し、不変性を保証してください。
79-85:chatRoomsRepositoryにreadonly修飾子を追加してください。SonarCloud が指摘しているように、再代入されないメンバーは
readonlyとしてマークすべきです。これは不変性を保証し、コードの安全性を向上させます。♻️ 修正案
constructor( `@Inject`(DI.chatRoomsRepository) - private chatRoomsRepository: ChatRoomsRepository, + private readonly chatRoomsRepository: ChatRoomsRepository, private chatService: ChatService, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/chat-room.ts` around lines 79 - 85, コンストラクタの注入済みプロパティ chatRoomsRepository（型 ChatRoomsRepository、デコレータ `@Inject`(DI.chatRoomsRepository) が付いたパラメータ）に readonly 修飾子を追加してください（例: private readonly chatRoomsRepository: ChatRoomsRepository）して再代入を防ぎ、他のパラメータやデコレータは変更しないでください。packages/backend/src/server/api/stream/channels/antenna.ts (1)
27-31:readonly修飾子を追加してください。静的解析ツールが指摘しているように、
antennasRepositoryとnoteStreamingHidingServiceは再代入されないため、readonly修飾子を付けるべきです。♻️ 修正案
constructor( - private antennasRepository: AntennasRepository, + private readonly antennasRepository: AntennasRepository, private roleService: RoleService, private noteEntityService: NoteEntityService, - private noteStreamingHidingService: NoteStreamingHidingService, + private readonly noteStreamingHidingService: NoteStreamingHidingService, id: string, connection: Channel['connection'], ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/antenna.ts` around lines 27 - 31, クラスのコンストラクタ注入フィールドで再代入されないため、antennasRepository（型 AntennasRepository）と noteStreamingHidingService（型 NoteStreamingHidingService）に readonly 修飾子を追加してください；具体的には該当するコンストラクタ引数／プロパティ宣言（antennasRepository, noteStreamingHidingService を受け取っている箇所、及び同名のクラスフィールド）に readonly を付けて不変性を明示してください。packages/backend/src/server/api/stream/channels/home-timeline.ts (4)
28-28:readonly修飾子を追加してください。
noteStreamingHidingServiceは再代入されないため、readonlyとしてマークすべきです。これは静的解析ツールでも指摘されています。🔧 提案する修正
- private noteStreamingHidingService: NoteStreamingHidingService, + private readonly noteStreamingHidingService: NoteStreamingHidingService,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/home-timeline.ts` at line 28, The property declaration for noteStreamingHidingService should be marked readonly since it is never reassigned; update the constructor parameter/property declaration (private noteStreamingHidingService: NoteStreamingHidingService) to include the readonly modifier (private readonly noteStreamingHidingService: NoteStreamingHidingService) so the field is immutable and satisfies static analysis.
87-92: null安全性のパターンが一貫していません。48-49行目で
userが非nullであることを確認した後、87行目と91行目ではthis.user?.idを使用していますが、他の箇所（72行目、96行目、101行目）ではuser.idを直接使用しています。早期リターン後は
userが存在することが保証されているため、一貫してuser.idを使用すべきです。♻️ 提案する修正
- if (!(await this.noteEntityService.isLanguageVisibleToMe(note, this.user?.id))) return; + if (!(await this.noteEntityService.isLanguageVisibleToMe(note, user.id))) return; if (this.isNoteMutedOrBlocked(note)) return; - const { shouldSkip } = await this.noteStreamingHidingService.processHiding(note, this.user?.id ?? null); + const { shouldSkip } = await this.noteStreamingHidingService.processHiding(note, user.id); if (shouldSkip) return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/home-timeline.ts` around lines 87 - 92, The null-safety usage is inconsistent: after earlier checks guarantee this.user is non-null, replace optional chaining with direct access to user.id to be consistent and avoid redundant ?. Specifically, update the calls to this.noteEntityService.isLanguageVisibleToMe(note, this.user?.id) and this.noteStreamingHidingService.processHiding(note, this.user?.id ?? null) to use this.user.id (and pass null only where appropriate), ensuring all uses in this file consistently reference user.id (e.g., align with other usages at the surrounding lines).
70-73:else ifに統合して可読性を向上させてください。
elseブロック内に単一のif文のみが存在します。else ifに統合することでネストを減らせます。♻️ 提案する修正
if (this.following[note.userId]?.withReplies) { if (!this.isNoteVisibleForMe(reply)) return; - } else { - // 「チャンネル接続主への返信」でもなければ、「チャンネル接続主が行った返信」でもなければ、「投稿者の投稿者自身への返信」でもない場合 - if (reply.userId !== user.id && !isMe && reply.userId !== note.userId) return; + } else if (reply.userId !== user.id && !isMe && reply.userId !== note.userId) { + // 「チャンネル接続主への返信」でもなければ、「チャンネル接続主が行った返信」でもなければ、「投稿者の投稿者自身への返信」でもない場合 + return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/home-timeline.ts` around lines 70 - 73, Replace the nested else { if (...) return; } with a single else if to reduce nesting and improve readability: in the block containing the check using reply.userId, user.id, isMe and note.userId, change the pattern "else { if (reply.userId !== user.id && !isMe && reply.userId !== note.userId) return; }" to "else if (reply.userId !== user.id && !isMe && reply.userId !== note.userId) return;" so the condition is evaluated as part of the original if/else-if chain and not as an inner nested if.
136-136:readonly修飾子を追加してください。
HomeTimelineChannelと同様に、このプロパティもreadonlyとしてマークすべきです。🔧 提案する修正
- private noteStreamingHidingService: NoteStreamingHidingService, + private readonly noteStreamingHidingService: NoteStreamingHidingService,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/home-timeline.ts` at line 136, The constructor/property declaration for noteStreamingHidingService should be marked readonly like in HomeTimelineChannel; update the class (where noteStreamingHidingService is declared) to add the readonly modifier to the private noteStreamingHidingService property so it becomes private readonly noteStreamingHidingService, ensuring immutability consistent with HomeTimelineChannel.packages/backend/src/server/api/stream/channels/global-timeline.ts (2)
25-30: 新規追加の DI メンバーはreadonlyで十分です。
noteStreamingHidingServiceは再代入されていないので、可変にしておく理由がありません。依存の不変性を型で表した方が意図が明確です。diff
- private noteStreamingHidingService: NoteStreamingHidingService, + private readonly noteStreamingHidingService: NoteStreamingHidingService,Also applies to: 136-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/global-timeline.ts` around lines 25 - 30, コンストラクタで注入されている依存のうち再代入されていないもの（例: noteStreamingHidingService や同様の新規 DI メンバー）を mutable にしているため意図が不明瞭です。constructor のパラメータ宣言（constructor(..., private noteStreamingHidingService: NoteStreamingHidingService, ...)）および同様の箇所（該当する他の DI メンバー）を readonly に変更して依存を不変にし、再代入が不要であることを型で明示してください。
39-50:init()の戻り値はPromise<boolean>を明示した方が安全です。ここは
false/trueで初期化可否を返す契約になったので、推論任せにせず型で固定しておくと将来の戻り値の取りこぼしをコンパイル時に拾えます。diff
- public async init(params: JsonObject) { + public async init(params: JsonObject): Promise<boolean> {注記: この同じパターンは他のタイムラインチャネル（local-timeline など）でも見られます。全体的な一貫性向上として複数のチャネルで同時に適用することを検討してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/global-timeline.ts` around lines 39 - 50, The init method lacks an explicit return type; change the signature of init to declare Promise<boolean> so the compiler enforces the true/false contract (e.g., update the declaration of init in the class that calls roleService.getUserPolicies and sets this.withRenotes/this.withFiles/minimize and subscribes this.subscriber.on('notesStream', this.onNote)); ensure any other timeline channel methods with the same pattern (e.g., local-timeline init variants) are updated similarly to maintain consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/server/api/stream/channels/hybrid-timeline.ts`:
- Around line 104-109: The handler mutates the shared note object by assigning
note.renote.myReaction, which leaks state across connections; instead create a
shallow copy of the note or of note.renote before adding myReaction (or build a
minimal payload object like the minimize block does) and pass that copy to
this.send('note', ...). Concretely, replace direct assignment using note.renote
and note.myReaction with creating a newRenote = { ...note.renote, myReaction:
myRenoteReaction } (or noteCopy = { ...note, renote: newRenote }) and send
noteCopy; continue to use noteEntityService.populateMyReaction(...) to compute
myRenoteReaction but never mutate the original note object.
In `@packages/backend/src/server/api/stream/channels/user-list.ts`:
- Around line 68-72: The interval handler for listUsersClock can start
overlapping updateListUsers() runs; add an in-flight guard (e.g.
this.isUpdatingListUsers) so the interval callback first checks the flag and
returns if true, sets it to true before calling updateListUsers(), and clears it
in a finally block to ensure it resets even on errors so concurrent executions
are prevented; keep the existing error handling that calls
this.connection.disconnectChannel(this.id) when updateListUsers() throws.
---
Nitpick comments:
In `@packages/backend/src/core/entities/NoteEntityService.ts`:
- Around line 148-197: The shouldHideNote method has high cognitive complexity
and uses .some(id => meId === id) where .includes(meId) is clearer; refactor by
extracting small helper functions (e.g., isSpecifiedVisibleTo(packedNote, meId),
isFollowerVisibleTo(packedNote, meId), and isSigninOrTimeHidden(packedNote,
meId)) to encapsulate each visibility branch and return early, and replace
occurrences of packedNote.visibleUserIds?.some(id => meId === id) and
packedNote.mentions?.some(id => meId === id) with
packedNote.visibleUserIds?.includes(meId) and
packedNote.mentions?.includes(meId) respectively to reduce complexity and
improve readability while keeping the public async shouldHideNote signature
unchanged.
In `@packages/backend/src/server/api/stream/channels/antenna.ts`:
- Around line 27-31: クラスのコンストラクタ注入フィールドで再代入されないため、antennasRepository（型
AntennasRepository）と noteStreamingHidingService（型 NoteStreamingHidingService）に
readonly 修飾子を追加してください；具体的には該当するコンストラクタ引数／プロパティ宣言（antennasRepository,
noteStreamingHidingService を受け取っている箇所、及び同名のクラスフィールド）に readonly を付けて不変性を明示してください。
In `@packages/backend/src/server/api/stream/channels/chat-room.ts`:
- Around line 22-30: コンストラクタの注入パラメータである chatRoomsRepository
が再代入されないことを明示するため、constructor のパラメータ宣言で chatRoomsRepository に readonly
修飾子を追加してください（対象シンボル: chatRoomsRepository, ChatRoomsRepository, constructor in
chat-room.ts）。他の注入済みパラメータ（例: chatService）と同様に readonly として宣言し、不変性を保証してください。
- Around line 79-85: コンストラクタの注入済みプロパティ chatRoomsRepository（型
ChatRoomsRepository、デコレータ `@Inject`(DI.chatRoomsRepository) が付いたパラメータ）に readonly
修飾子を追加してください（例: private readonly chatRoomsRepository:
ChatRoomsRepository）して再代入を防ぎ、他のパラメータやデコレータは変更しないでください。
In `@packages/backend/src/server/api/stream/channels/global-timeline.ts`:
- Around line 25-30: コンストラクタで注入されている依存のうち再代入されていないもの（例:
noteStreamingHidingService や同様の新規 DI メンバー）を mutable にしているため意図が不明瞭です。constructor
のパラメータ宣言（constructor(..., private noteStreamingHidingService:
NoteStreamingHidingService, ...)）および同様の箇所（該当する他の DI メンバー）を readonly
に変更して依存を不変にし、再代入が不要であることを型で明示してください。
- Around line 39-50: The init method lacks an explicit return type; change the
signature of init to declare Promise<boolean> so the compiler enforces the
true/false contract (e.g., update the declaration of init in the class that
calls roleService.getUserPolicies and sets
this.withRenotes/this.withFiles/minimize and subscribes
this.subscriber.on('notesStream', this.onNote)); ensure any other timeline
channel methods with the same pattern (e.g., local-timeline init variants) are
updated similarly to maintain consistency.
In `@packages/backend/src/server/api/stream/channels/home-timeline.ts`:
- Line 28: The property declaration for noteStreamingHidingService should be
marked readonly since it is never reassigned; update the constructor
parameter/property declaration (private noteStreamingHidingService:
NoteStreamingHidingService) to include the readonly modifier (private readonly
noteStreamingHidingService: NoteStreamingHidingService) so the field is
immutable and satisfies static analysis.
- Around line 87-92: The null-safety usage is inconsistent: after earlier checks
guarantee this.user is non-null, replace optional chaining with direct access to
user.id to be consistent and avoid redundant ?. Specifically, update the calls
to this.noteEntityService.isLanguageVisibleToMe(note, this.user?.id) and
this.noteStreamingHidingService.processHiding(note, this.user?.id ?? null) to
use this.user.id (and pass null only where appropriate), ensuring all uses in
this file consistently reference user.id (e.g., align with other usages at the
surrounding lines).
- Around line 70-73: Replace the nested else { if (...) return; } with a single
else if to reduce nesting and improve readability: in the block containing the
check using reply.userId, user.id, isMe and note.userId, change the pattern
"else { if (reply.userId !== user.id && !isMe && reply.userId !== note.userId)
return; }" to "else if (reply.userId !== user.id && !isMe && reply.userId !==
note.userId) return;" so the condition is evaluated as part of the original
if/else-if chain and not as an inner nested if.
- Line 136: The constructor/property declaration for noteStreamingHidingService
should be marked readonly like in HomeTimelineChannel; update the class (where
noteStreamingHidingService is declared) to add the readonly modifier to the
private noteStreamingHidingService property so it becomes private readonly
noteStreamingHidingService, ensuring immutability consistent with
HomeTimelineChannel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f0420d3-b343-4d50-a9ea-2e13255588ca
📒 Files selected for processing (8)
packages/backend/src/core/entities/NoteEntityService.tspackages/backend/src/server/api/stream/channels/antenna.tspackages/backend/src/server/api/stream/channels/chat-room.tspackages/backend/src/server/api/stream/channels/global-timeline.tspackages/backend/src/server/api/stream/channels/home-timeline.tspackages/backend/src/server/api/stream/channels/hybrid-timeline.tspackages/backend/src/server/api/stream/channels/local-timeline.tspackages/backend/src/server/api/stream/channels/user-list.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/backend/src/server/api/stream/channels/role-timeline.ts (1)
36-43:initの戻り値型が暗黙的です。
initメソッドがbooleanを返すようになりましたが、TypeScript の戻り値型アノテーションがありません。他のチャネル（local-timeline.ts、global-timeline.ts）ではPromise<boolean>を明示的に指定しています。一貫性のために型アノテーションを追加することを推奨します。♻️ 修正案
`@bindThis` - public async init(params: JsonObject) { + public async init(params: JsonObject): Promise<boolean> { if (typeof params.roleId !== 'string') return false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/role-timeline.ts` around lines 36 - 43, The init method currently returns a boolean implicitly; update its signature to explicitly declare the return type Promise<boolean> (i.e., change async init(params: JsonObject) to async init(params: JsonObject): Promise<boolean>) so it matches other channels; no logic changes are needed—keep the roleId and minimize assignments and the subscriber.on(`roleTimelineStream:${this.roleId}`, this.onEvent) call, and ensure the method still returns true/false as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/backend/src/server/api/stream/channels/role-timeline.ts`:
- Around line 36-43: The init method currently returns a boolean implicitly;
update its signature to explicitly declare the return type Promise<boolean>
(i.e., change async init(params: JsonObject) to async init(params: JsonObject):
Promise<boolean>) so it matches other channels; no logic changes are needed—keep
the roleId and minimize assignments and the
subscriber.on(`roleTimelineStream:${this.roleId}`, this.onEvent) call, and
ensure the method still returns true/false as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eaa556ad-4b72-47bb-b3ce-32ffddfcdaca
📒 Files selected for processing (9)
packages/backend/src/core/entities/NoteEntityService.tspackages/backend/src/server/api/stream/channels/antenna.tspackages/backend/src/server/api/stream/channels/chat-room.tspackages/backend/src/server/api/stream/channels/global-timeline.tspackages/backend/src/server/api/stream/channels/home-timeline.tspackages/backend/src/server/api/stream/channels/hybrid-timeline.tspackages/backend/src/server/api/stream/channels/local-timeline.tspackages/backend/src/server/api/stream/channels/role-timeline.tspackages/backend/src/server/api/stream/channels/user-list.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/backend/src/server/api/stream/channels/chat-room.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/core/entities/NoteEntityService.ts`:
- Around line 178-179: The current return uses only
userFollowingsCache.fetch(meId) and Object.hasOwn to decide visibility, dropping
the existing fallback in isVisibleForMe; update the check in NoteEntityService
so it returns true if the cached follow exists OR if the fallback applies (both
poster and viewer are remote and follow relationship cannot be confirmed),
preserving the original behavior in isVisibleForMe (lines 326-345). Concretely,
after fetching followings with userFollowingsCache.fetch(meId) and checking
Object.hasOwn(followings, packedNote.userId), add the fallback branch that
detects remote users (use your existing remote-check helper or the same
conditions used in isVisibleForMe) and returns true when both sides are remote
and no follow info is available. Ensure you reference packedNote.userId, meId,
userFollowingsCache.fetch and the isVisibleForMe fallback logic so the
visibility decision remains consistent.
- Around line 170-172: isFollowerVisibleTo() currently relies on
packedNote.reply?.userId which can be omitted when pack() is called with detail:
false (see pack() behavior around lines 526-533), causing followers-only replies
to be misclassified; update isFollowerVisibleTo() (and the similar check at the
other occurrence around lines 197-199) to determine the reply target from data
that does not depend on reply being packed—e.g., read the reply user id from the
original Note object (note.reply?.userId) or from a stable replyId/replyToUserId
field on the packed payload (fallback to packedNote.replyToUserId if present) so
the followers-exception logic in shouldHideNote()/isVisibleForMe() behaves
consistently regardless of pack(detail:false).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 83609195-200c-4825-a6cc-a0bcdb79a3f4
📒 Files selected for processing (1)
packages/backend/src/core/entities/NoteEntityService.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/backend/src/server/api/stream/channels/role-timeline.ts (1)
36-43: initメソッドの型注釈のスペーシングに軽微な不整合があります。TypeScriptの一般的なスタイルでは、戻り値の型注釈でコロンの前にスペースを入れません。
♻️ スタイル修正の提案
- public async init(params: JsonObject) : Promise<boolean> { + public async init(params: JsonObject): Promise<boolean> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/role-timeline.ts` around lines 36 - 43, Signature spacing in init is inconsistent: change the return-type annotation in the init method from "init(params: JsonObject) : Promise<boolean>" to remove the extra space before the colon so it reads "init(params: JsonObject): Promise<boolean>" in the role-timeline.ts file; update any other method signatures nearby with the same spacing if present and run the project's TypeScript/ESLint formatter to ensure consistent style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/backend/src/server/api/stream/channels/role-timeline.ts`:
- Around line 36-43: Signature spacing in init is inconsistent: change the
return-type annotation in the init method from "init(params: JsonObject) :
Promise<boolean>" to remove the extra space before the colon so it reads
"init(params: JsonObject): Promise<boolean>" in the role-timeline.ts file;
update any other method signatures nearby with the same spacing if present and
run the project's TypeScript/ESLint formatter to ensure consistent style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 42c73e51-9062-4dca-bac8-dda28da2e4f9
📒 Files selected for processing (1)
packages/backend/src/server/api/stream/channels/role-timeline.ts
mattyatea
force-pushed
the
hotfix/securty-patch
branch
from
3c017e5 to
e511460
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/backend/src/server/api/stream/channels/role-timeline.ts (2)
134-134:readonly修飾子を追加してください。Line 26 と同様に、このメンバーも再代入されないため
readonlyを付けるべきです。🔧 修正案
- private noteStreamingHidingService: NoteStreamingHidingService, + private readonly noteStreamingHidingService: NoteStreamingHidingService,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/role-timeline.ts` at line 134, noteStreamingHidingService メンバーは再代入されないため readonly を付けて不変であることを明示してください。該当のコンストラクタまたはクラスフィールド宣言で noteStreamingHidingService の宣言に readonly 修飾子を追加し、他のフィールドと同様の扱い（例: Line 26 と同様）に揃えてください。
26-26:readonly修飾子を追加してください。静的解析ツール（SonarCloud）が指摘している通り、
noteStreamingHidingServiceは再代入されないためreadonlyを付けるべきです。🔧 修正案
- private noteStreamingHidingService: NoteStreamingHidingService, + private readonly noteStreamingHidingService: NoteStreamingHidingService,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/role-timeline.ts` at line 26, noteStreamingHidingService が再代入されないため readonly 修飾子を付けて不変性を明示してください；クラスのコンストラクタ引数やプロパティ宣言で宣言されている noteStreamingHidingService を見つけ（例: コンストラクタのパラメータ一覧やクラスフィールド）、型宣言の前に readonly を追加して再代入を防止してください（例えば "private noteStreamingHidingService: NoteStreamingHidingService" を "private readonly noteStreamingHidingService: NoteStreamingHidingService" に変更）。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/backend/src/server/api/stream/channels/role-timeline.ts`:
- Line 134: noteStreamingHidingService メンバーは再代入されないため readonly
を付けて不変であることを明示してください。該当のコンストラクタまたはクラスフィールド宣言で noteStreamingHidingService の宣言に
readonly 修飾子を追加し、他のフィールドと同様の扱い（例: Line 26 と同様）に揃えてください。
- Line 26: noteStreamingHidingService が再代入されないため readonly
修飾子を付けて不変性を明示してください；クラスのコンストラクタ引数やプロパティ宣言で宣言されている noteStreamingHidingService
を見つけ（例: コンストラクタのパラメータ一覧やクラスフィールド）、型宣言の前に readonly を追加して再代入を防止してください（例えば "private
noteStreamingHidingService: NoteStreamingHidingService" を "private readonly
noteStreamingHidingService: NoteStreamingHidingService" に変更）。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2732f79d-e816-4355-ab5f-53c2edf09845
📒 Files selected for processing (2)
packages/backend/src/core/entities/NoteEntityService.tspackages/backend/src/server/api/stream/channels/role-timeline.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/backend/src/server/api/stream/channels/role-timeline.ts (1)
96-111:cacheNote()にnoteToSendを渡して、キャッシュ と配信ペイロードを揃えることを検討してください。この変更で配信本体は
noteToSendを使用していますが、キャッシュだけ元のnoteのままです。cachedNotesは private で直接的な読み取りメソッドが見当たらないため、現在のコードでは実際の影響が生じていない可能性が高いですが、将来的にキャッシュが消費される場合に備えて、両者の整合性を取っておくことが望ましいです。
もしcachedNotesが実際には使用されていない場合、このキャッシュ機構自体の必要性を検討する価値があります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/server/api/stream/channels/role-timeline.ts` around lines 96 - 111, The cache is being populated with the original note object while the payload sent uses noteToSend, causing potential divergence; change the call inside the if (this.user && ...) block from this.connection.cacheNote(note) to this.connection.cacheNote(noteToSend) so cachedNotes matches what is emitted, and if noteToSend may be undefined in that scope add a safe fallback (cache noteToSend ?? note) to preserve existing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/server/api/stream/channels/role-timeline.ts`:
- Around line 65-70: The code handling "pure renote" cases currently only checks
note.renote.reply and misses validating the inner renote itself and the reply
author's requireSigninToViewContents; update the branch inside the
isRenotePacked && !isQuotePacked check to first call
isNoteVisibleForMe(note.renote) and return if it is not visible, then if
note.renote.reply exists check isNoteVisibleForMe(note.renote.reply) and also
verify note.renote.reply.user.requireSigninToViewContents (and/or
note.renote.user.requireSigninToViewContents as appropriate) before proceeding
so followers/direct-only inner renotes are not emitted erroneously.
---
Nitpick comments:
In `@packages/backend/src/server/api/stream/channels/role-timeline.ts`:
- Around line 96-111: The cache is being populated with the original note object
while the payload sent uses noteToSend, causing potential divergence; change the
call inside the if (this.user && ...) block from this.connection.cacheNote(note)
to this.connection.cacheNote(noteToSend) so cachedNotes matches what is emitted,
and if noteToSend may be undefined in that scope add a safe fallback (cache
noteToSend ?? note) to preserve existing behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 938688a2-965b-48dd-af43-6df52963edf6
📒 Files selected for processing (1)
packages/backend/src/server/api/stream/channels/role-timeline.ts
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fix: セキュリティに関する修正
CVE-2026-28431
GHSA-r33c-qg3g-v9cr
GHSA-cvf3-p7p2-27fh
GHSA-gg7j-c76w-8x3g
CVE-2026-28432
GHSA-grwc-c762-gcvp
CVE-2026-28433
GHSA-g6hj-33h7-6fq8
Why
Additional info (optional)
Checklist
Summary by CodeRabbit
バグ修正
新機能
リファクター
その他