Skip to content

Fix/issue62782/issue62796(No.557, 560, 618, 1016) - #1932

Merged
ivis-miyachi merged 4 commits into
RCOSDP:develop_v2.1.0from
KotaroInoue1448:fix/issue62782/issue62796
Sep 29, 2026
Merged

ivis-miyachi merged 4 commits into
RCOSDP:develop_v2.1.0from
KotaroInoue1448:fix/issue62782/issue62796

Conversation

@KotaroInoue1448

@KotaroInoue1448 KotaroInoue1448 commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

概要 (Summary)

関連Issue / チケット (Related Issues)

  • close #

変更タイプ (Type of Change)

  • 🚀 新機能追加 (Feature)
  • 🐛 バグ修正 (Bug Fix)
  • 🔒 セキュリティ修正 (Security Fix)
  • 🚫 機能のクローズ・非公開化・削除 (Feature Deprecation/Disable)
  • ⚠️ 破壊的変更・データ移行を伴う修正 (Breaking Change / Migration)
  • 📚 仕様書・マニュアル・APIリストの更新 (Documentation)

🤖 0. CI 自動チェック (API Inventory Drift)

PR ごとに WEKO3 コンテナを起動し、url_map のダンプ・台帳との突き合わせ・変更行の
到達可否測定を自動実行する。結果は PR コメントと Actions の artifact
(api-inventory-summary) に出る。

このリポジトリは public のため、台帳もベースラインも同梱していない。
実データはプライベートリポジトリ RCOSDP/weko-secret にあり、CI は Secret 経由で取得する。
以降この文書では、そこを単にプライベートリポジトリと呼ぶ。

台帳はブランチごとに内容が違うため、CI は weko 側と同名のブランチを
プライベートリポジトリから探して使う(head → base → 既定ブランチ の順)。
採用されたブランチ名は PR コメントの冒頭に出るので、件数を読む前にそこを見ること。
対応ブランチが無い場合は既定ブランチと比較され、コメント冒頭に警告が出る。
その件数は当てにならないので、PASS でも「確認済み」と読まないこと。
詳細: tools/api-inventory/ci/README.md §3a

  • CI が PASS している、または FAIL の各項目に対処済み
    Secret (API_INVENTORY_REPO / API_INVENTORY_SSH_KEY) が未設定のリポジトリ、
    および fork からの PR では、このジョブは何もせずスキップされる。

API を追加・変更した場合(必須)

  • プライベートリポジトリ側の作業ブランチを、この PR のブランチと同名で切った
    この PR が fix/issue62569 → develop_v2.0.4 なら、プライベート側も
    fix/issue62569 → develop_v2.0.4。同名にしておけば台帳 PR が未マージでも
    CI がそれを見るので、2つの PR のマージ順を気にしなくてよい。
  • プライベートリポジトリの api_snapshot.json を更新し、対応する PR を出した
    bash export WEKO_API_INVENTORY_DIR=/path/to/weko-secret ./install.sh python3 tools/api-inventory/scripts/snapshot.py --out "$WEKO_API_INVENTORY_DIR/api_snapshot.json"
    更新しないと CI が落ちる。公開リポジトリのコード変更とは別の PRになる。
  • プライベートリポジトリの weko3_api_list_full.tsv に行を追加・更新し、
    build_checklist.py で 24 列版を再生成した(未収載だと reconcile が FAIL する)
  • 台帳・スナップショット・実測結果をこの公開リポジトリにコミットしていない
    (git status に *.tsv / api_snapshot.json が出ていないこと)

FAIL したときの対処(要約)

まず PR コメント冒頭の台帳ブランチを見る。警告が出ていれば、件数を追う前に
プライベートリポジトリ側の対応ブランチを用意すること(比較相手が違うので件数に意味がない)。

ジョブが落ちる条件は 3 つある。PR コメントのどのセクションに件数が出ているかで切り分ける。

落ちた場所 落ちる条件
ベースラインとの差分 (drift.md) G1〜G7 のいずれかに該当
台帳との突き合わせ (reconcile.md) A + B + C + D + E の合計が 1 件以上
変更行の到達可否測定 (probe) G8 / G9 に該当(結果は artifact に含めないので Actions のログで件数を見る)
検出 意味 対処
G1 / G2 新規経路に認証系デコレータが無い / 認証系デコレータが削除された 実装を直す。意図的な公開なら台帳に根拠を書いてベースライン更新
G3 認証・認可デコレータのコメントアウトが増えた 原則やり直し。残す場合は理由をコード中のコメントに明記する
G4 *_PERMISSION_FACTORY / CSRF 保護 等が危険側の値に変わった 原則やり直し
G5 ModelView の can_delete / can_export が False → True 意図的なら台帳の data_op を更新
G6 静的解析で属性が取れない経路が追加された 台帳に行を追加してレビューする(外部ライブラリ由来など)
G7 依存パッケージの更新で経路が増減した 増えた経路は台帳に追加。消えた経路は行を削除するか allow に登録
G8 台帳で data_op が作成/更新/削除の経路に、未認証で到達した 原則やり直し。意図的な公開なら台帳の根拠を更新。data_op の記載誤りなら台帳を直す
G9 台帳では遮断なのに実測で到達(認可の回帰) 原則やり直し
reconcile A / E 実機にあるが台帳に無い(A: URI 単位 / E: 同一 URI の endpoint 単位) 台帳に行を追加する
reconcile B 台帳にあるが実機の url_map に無い 理由を確認し、正当なら allow に登録する
reconcile C / D メソッド・app 列の記載誤り 台帳を実機に合わせる

reconcile B のうち、実機に存在しないことが正当な行(プラグイン未登録・config で無効等)は
プライベートリポジトリの reconcile_allow.json に理由付きで登録する。理由なしの登録は不可。
登録済みの行は B'(既知・許容)として集計され、E'(endpoint が実機に無い)と併せてゲート対象外になる。

W1〜W6 は WARN でゲートは通るが、レビューでは見ること
(ModelView の追加 / 実装本体の変化 / HTTP メソッド・URL の変化 / 監視対象 config の変化 /
依存パッケージの版の変化)。特に W6(依存の版)は、ベースラインを CI と異なる環境で作ると
毎回出続けて形骸化するため、ベースラインは install.sh で作った環境から生成する。

CI の出力は件数のみ。該当した経路名は Actions には出ないので、プライベートリポジトリ側の
完全版レポート(--summary-only なしで再実行したもの)で確認すること
(このリポジトリは public で、ログ・artifact・PR コメントは誰でも読めるため)。


🔒 1. セキュリティ & API アクセス制御チェック (必須)

認証・認可 (Authentication & Authorization)

  • 新規/変更された Blueprint・View・REST リソースに適切なデコレータ / Permission を設定している
    • 例: @login_required, @pass_record, need(...), Invenio Access Action
    • /api/* では Permission.require(http_exception=403) を使うこと。
      @login_required は API アプリに security.login が無いため 401 ではなく 500 になる
    • CI: G1 / G2 が自動検出(デコレータの有無・削除)
  • 状態変更・破壊的メソッド (POST / PUT / PATCH / DELETE) の権限が正しく制限されている
    • CI: G8 が変更行を未認証で実測(使い捨て環境なので --allow-writes 付きで
      GET / HEAD 以外も叩く)
  • 未ログイン(Anonymous)状態でアクセスした際、意図しないデータ取得・変更が拒絶される
    • CI: G8 / G9 が変更行を実測。ただし測定対象は変更行のみ、かつ既定プロファイルで
      起動した経路のみ。ワークフロー系など未解決プレースホルダの行は skip される
  • 認可を config の permission factory に委ねている場合、None で無効化していない
    • CI: G4 が *_PERMISSION_FACTORY 等を監視

機能クローズ・非公開化の場合 (Feature Disable)

  • UI(画面・ボタン)の非表示だけでなく、バックエンド API(ルーティング・View)も完全に遮断されている
  • 無効化状態で直接 API を叩いた場合、404 Not Found または 403 Forbidden が返ることを確認した

🧪 2. テストコード観点チェック (pytest / Invenio Test Suite)

権限・異常系テスト (Negative & Authorization Tests)

  • 未認証アクセス (Anonymous): トークン/セッションなしのリクエストで 401 Unauthorized または 403 Forbidden / 404 Not Found が返ることを検証するテストがある
  • 権限不足ユーザー (Forbidden): 閲覧権限のみのユーザーが更新/削除 API を叩いた際に 403 になるテストがある
  • 無効化/非公開機能の遮断テスト: 対象機能が無効化されている場合、エンドポイントが 404 / 403 を返すテストがある

境界値・入力バリデーションテスト (Boundary & Validation)

  • 不正なパラメータ(巨大ファイル、異常な MIME タイプ、無効な JSON/XML スキーマ、SQLi/XSS ペイロード等)で適切に 400 Bad Request / バリデーションエラーが返るテストがある

データ整合性・トランザクションテスト (Integrity & Rollback)

  • ファイルストレージ(S3/ローカル)書き込み失敗時や DB エラー時に、中途半端なレコードやゴミファイルが残らずロールバックされるテストがある

🛡️ 3. データ保護 & 破壊的変更防止チェック (Data Safety)

  • 物理削除・上書きの安全性:
    • ファイル・アイテム・メタデータの完全削除/置換処理に、意図しない一括削除や別レコードへの誤適用リスクがない
    • 論理削除、バージョン管理、バックアップ等のロールバック機構が考慮されている
  • トランザクション整合性:
    • DB 更新とストレージ操作がアトミックに管理されている

⚙️ 4. マイグレーション & システム影響チェック (Invenio / WEKO3 Stack)

データベース (DB / Alembic)

  • invenio alembic upgrade(適用)および downgrade(ロールバック)スクリプトを作成・検証した
  • 既存データに対する破壊的変更(カラム削除、型変更、NOT NULL 制約追加等)の移行スクリプト/データパッチを用意した

検索インデックス (Elasticsearch / OpenSearch)

  • マッピング定義変更の有無を確認した
  • インデックス再作成(Reindex)やエイリアス切り替え手順を準備・検証した

設定 & 非同期処理 (Config / Celery / Cache)

  • invenio.cfg / 環境変数のデフォルト値を設定した
  • Celery タスクのシグネチャ変更によるキュー滞留・不整合が発生しない
  • キャッシュ(Redis/Memcached)のパージが必要か確認した

📚 5. ドキュメント・仕様書更新チェック (weko-document)

  • API インベントリ: ツールは本リポジトリの tools/api-inventory/、
    台帳・調査記録はプライベートリポジトリ(public リポジトリには置かない)。
    §0 のチェック項目で対応済みなら、ここは確認のみ。
    • エンドポイントの追加・変更・廃止、メソッド、認証・認可要件、リクエスト/レスポンス仕様を更新した
    • 調査記録(weko3_api_auth_findings.md)もプライベートリポジトリに置く。台帳は二重管理しない
  • WEKO3 機能仕様書:
    • 対象機能の仕様追加・変更・クローズ(非公開化)内容を反映した
  • 各種マニュアル (管理者 / 利用者マニュアル):
    • 画面導線・操作手順・権限仕様の変更を反映した
  • 更新不要な場合(理由):

📋 6. 動作検証エビデンス (Verification Evidence)

テスト実行結果

cd modules/<対象モジュール> && pytest
# -> PASS

CI の成果物 (artifact: api-inventory-summary)

ファイル 内容
drift.md ベースラインとの差分(件数のみ)
reconcile.md 台帳と実機の突き合わせ(件数のみ)

明細(該当した経路名・実測結果)は公開できないため artifact に含めていない。
プライベートリポジトリ側で同じコマンドを --summary-only なしで実行して確認する。

手動で確認したこと

Summary by Sourcery

Strengthen access control for index-based journal details and workflow feedback mail retrieval.

Bug Fixes:

  • Restrict journal detail access to indexes permitted for the current user, returning appropriate errors for invalid or unauthorized index requests.
  • Enforce authority checks when retrieving workflow feedback mail recipients.

Tests:

  • Add coverage for index permission validation and forbidden journal detail access.
  • Update workflow access-control tests to provide the required action permissions.

@sourcery-ai

sourcery-ai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Reviewer's Guide

Strengthens authorization for index-scoped search UI operations and workflow feedback-mail retrieval by adding role/authority checks, with coverage for unauthorized index access and updated workflow permission test setup.

Sequence diagram for index-scoped search authorization

sequenceDiagram
    participant Client
    participant SearchUI
    participant Indexes
    participant Roles

    Client->>SearchUI: journal_detail(index_id) or get_path_name_dict(path_str)
    SearchUI->>SearchUI: check_index_permission
    SearchUI->>Indexes: get_index(index_id)
    Indexes-->>SearchUI: index
    SearchUI->>Roles: filter_index_list_by_role(index_list)
    alt no permitted indexes
        Roles-->>SearchUI: empty list
        SearchUI-->>Client: 403 Forbidden
    else permitted indexes
        Roles-->>SearchUI: allowed indexes
        SearchUI->>SearchUI: journal_detail or get_path_name_dict
        SearchUI-->>Client: authorized response
    end
Loading

Sequence diagram for workflow feedback-mail authorization

sequenceDiagram
    participant Client
    participant Workflow

    Client->>Workflow: get_feedback_maillist(activity_id)
    Workflow->>Workflow: login_required
    alt authenticated user lacks authority
        Workflow->>Workflow: check_authority
        Workflow-->>Client: access denied
    else authorized user
        Workflow->>Workflow: check_authority
        Workflow-->>Client: feedback mail list
    end
Loading

File-Level Changes

Change Details Files
Adds index-level authorization enforcement to search UI endpoints.
  • Introduces a decorator that validates index identifiers, resolves indexes, filters them by the current user’s role, and returns 404/403 when appropriate.
  • Applies the decorator to journal detail and path-name lookup routes while preserving only authorized path components before view execution.
  • Adds unit and endpoint tests covering missing, invalid, nonexistent, unauthorized, and partially authorized indexes.
modules/weko-search-ui/weko_search_ui/utils.py
modules/weko-search-ui/weko_search_ui/views.py
modules/weko-search-ui/tests/test_utils.py
modules/weko-search-ui/tests/test_views.py
Adds authority checks to workflow feedback-recipient retrieval.
  • Decorates the feedback mail-list endpoint with workflow authority validation in addition to login enforcement.
  • Updates the related test fixture setup to provide the required full-action permissions.
modules/weko-workflow/weko_workflow/views.py
modules/weko-workflow/tests/test_views.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 0fccce32-595b-4224-af8a-74823a53d9bb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. This changes authorization behavior by filtering index paths and requiring authority for feedback-mail-list access. If the checks are wrong, unauthorized users could receive protected journal or workflow data, and reverting would not undo any access or disclosure that already occurred.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@ivis-miyachi
ivis-miyachi merged commit fa824f5 into RCOSDP:develop_v2.1.0 Sep 29, 2026
7 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.

2 participants