Repository navigation
Hotfix/v2.0.4 - #1954
Hotfix/v2.0.4#1954
Conversation
- records 系エンドポイントの permission factory を明示的に設定 - deposit エンドポイントにメソッド単位でデコレータを指定できる create_decorators / update_decorators オプションを追加 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- WEKO_WORKFLOW_GUEST_TOKEN_ISSUE_ENABLED / WEKO_WORKFLOW_GUEST_TOKEN_ACCEPT_ENABLED を追加（既定値は False） - scripts/instance.cfg で両設定を True に設定 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer's GuideThis hotfix hardens REST write access with method-specific authentication and authorization decorators, and adds configuration-driven controls that disable guest activity token issuance/acceptance and related workflows when turned off, while explicitly denying record mutations on key search endpoints. Sequence diagram for guest activity token gatingsequenceDiagram
participant Guest
participant WorkflowView
participant WorkflowUtils
participant Config
participant Session
Guest->>WorkflowView: POST /activity/init-guest
WorkflowView->>WorkflowUtils: guest_token_issue_required()
WorkflowUtils->>Config: is_guest_token_issue_enabled()
alt guest token issuance disabled
WorkflowUtils-->>Guest: GuestTokenDisabledError 403
else guest token issuance enabled
WorkflowView->>WorkflowUtils: init_activity_for_guest_user()
WorkflowUtils-->>Guest: Guest activity URL and token
end
WorkflowView->>WorkflowUtils: clear_guest_session()
WorkflowUtils->>Config: is_guest_token_accept_enabled()
alt guest token acceptance disabled
WorkflowUtils->>Session: pop guest_token, guest_email, guest_url
end
Sequence diagram for method-specific REST write authorizationsequenceDiagram
participant Client
participant DepositREST
participant DecoratedView
participant Auth
participant RecordAPI
Client->>DepositREST: POST create item
DepositREST->>DecoratedView: dispatch()
DecoratedView->>Auth: login_required_customize()
Auth-->>DecoratedView: authenticated request
DecoratedView->>RecordAPI: create item
RecordAPI-->>Client: response
Client->>DepositREST: PUT or PATCH update item
DepositREST->>DecoratedView: dispatch()
DecoratedView->>Auth: require_item_edit_permission()
Auth-->>DecoratedView: edit permission granted
DecoratedView->>RecordAPI: update item
RecordAPI-->>Client: response
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
🔍 Claude によるレビュー指摘 2 件 — 🔴 高 1 ／ 🟠 中 1 ／ 🟡 低 0 ／ 🔎 未確認 9 件 🔴 [高] 認証・編集権限のデコレータが deposit の actions / files ルートに及んでいない
提案 update_permission_factory_imp に edit_permission_factory 相当を設定する。または、actions・files ルートにも同じデコレータを適用する( 根拠確認: invenio_deposit/views/rest.py:130-230 を確認した。decorate_view は list_route と item_route にしか適用されず、DepositActionResource.post は @pass_record と @need_record_permission('update_permission_factory') だけで、require_api_auth は付いていない。invenio_records_rest/views.py:433-453 で、need_record_permission は factory が None だと検証を飛ばすことを確認した。weko_deposit/config.py:163 で RECORDS_REST_DEFAULT_UPDATE_PERMISSION_FACTORY=None であることを確認した。files 系の POST/PUT/DELETE には require_api_auth があるが、所有者・編集権限のチェックは無い。weko_deposit/rest.py:51-92 の require_item_edit_permission は item_route の PUT/PATCH にしか付かない。 🟠 [中] 認可デコレータの対象が一覧・アイテムのルートに限られ、files と actions の各ルートに認可が残らない
提案
根拠確認: invenio_deposit/views/rest.py:139-162 を確認。route_decorators のキーは list_route と item_route だけで、files/actions のルートは decorate_view を通らない。同 :233-475 を確認。DepositActionResource.post、DepositFilesResource.post/put、DepositFileResource.put/delete は 🔎 未確認 — 裏が取れなかったもの 9 件
モデル sonnet ／ 3 回実行して和集合 ／ コスト $0.7066。同じ差分でも結果が揺れるため複数回まわし、一部のパスでしか挙がらなかったものには回数を添えています 差分のみを対象にした自動レビューです。誤りが含まれることがあります。 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="modules/weko-deposit/weko_deposit/config.py" line_range="131" />
<code_context>
# read_permission_factory_imp='',
# update_permission_factory_imp='',
delete_permission_factory_imp=deny_all,
+ create_decorators=['weko_accounts.utils:login_required_customize'],
+ update_decorators=['weko_deposit.rest:require_item_edit_permission'],
)
</code_context>
<issue_to_address>
**Guests can create deposits**
When an unauthenticated visitor has a guest-token session and the create permission factory does not reject that visitor, `login_required_customize` treats the guest-token session as authenticated, so the new create decorator lets the request reach deposit creation without an authenticated account or an item-specific create check. The guest can create a deposit.
Require an authenticated account for deposit creation or add a create permission check that rejects guest-token sessions.
</issue_to_address>
### Comment 2
<location path="modules/weko-workflow/weko_workflow/utils.py" line_range="3564" />
<code_context>
+ bool: True if ``WEKO_WORKFLOW_GUEST_TOKEN_ISSUE_ENABLED`` is set.
+ """
+ return bool(current_app.config.get(
+ 'WEKO_WORKFLOW_GUEST_TOKEN_ISSUE_ENABLED', False))
+
+
</code_context>
<issue_to_address>
**String false enables guest tokens**
When an environment-specific config supplies either guest-token flag as the nonempty string `"False"`, `bool()` treats any nonempty string as true, so a setting supplied as the string `"False"` leaves the corresponding guest-token gate enabled. Both issuing and accepting tokens can therefore remain enabled when an operator intended to disable them.
Require actual boolean config values or parse supported string values explicitly and reject malformed values.
Also at `modules/weko-workflow/weko_workflow/utils.py:3574`.
</issue_to_address>
### Comment 3
<location path="modules/invenio-deposit/invenio_deposit/views/rest.py" line_range="139-144" />
<code_context>
'{0}/files/<path:key>'.format(options['item_route'])
)
+ route_decorators = {
+ options['list_route']: (
+ options.pop('create_decorators', []), ('POST',)),
+ options['item_route']: (
+ options.pop('update_decorators', []), ('PUT', 'PATCH')),
+ }
+
options.setdefault('search_class', DepositSearch)
</code_context>
<issue_to_address>
**File mutations skip edit checks**
When an authenticated API caller has the write scope but lacks permission to edit the deposit, `create_blueprint` attaches `update_decorators` only to the item route, while file routes are registered separately; a caller with the API write scope but no item edit permission can therefore add, rename, reorder, or delete deposit files.
Apply the item edit check to file mutation routes too, or configure an update permission factory that enforces per-record edit access there.
Also at `modules/weko-deposit/weko_deposit/config.py:132`.
</issue_to_address>
### Comment 4
<location path="modules/weko-admin/weko_admin/utils.py" line_range="1995-1997" />
<code_context>
.get("WEKO_WORKFLOW_REQUEST_FOR_REGISTER_USAGE_REPORT")
for activity in activities:
+ if (activity.extra_info or {}).get('is_guest') \
+ and not self.__is_guest_token_issue_enabled():
+ continue
if activity.item_id:
records_id.append(activity.item_id)
</code_context>
<issue_to_address>
**Skipped guests trigger other reminder emails**
When the activities contain a registered-user activity followed by a guest activity while guest token issuing is disabled, `send_reminder_mail` skips a disabled guest activity but leaves it in `activity`; the later guest check at line 2027 still runs and sends every accumulated entry in `self.__mail_info_lst`, including registered-user reminders that would not be sent without `forced_send`. The caller can therefore trigger unintended reminder emails.
Track whether any eligible guest activity was processed, and only use that state to trigger guest reminder sending.
</issue_to_address>
### Comment 5
<location path="modules/weko-admin/weko_admin/utils.py" line_range="1995-1996" />
<code_context>
.get("WEKO_WORKFLOW_REQUEST_FOR_REGISTER_USAGE_REPORT")
for activity in activities:
+ if (activity.extra_info or {}).get('is_guest') \
+ and not self.__is_guest_token_issue_enabled():
+ continue
if activity.item_id:
</code_context>
<issue_to_address>
**Reminders send unusable links**
When guest token issuance is enabled, acceptance is disabled, and a guest activity is due for a reminder, `UsageReport.send_reminder_mail` checks only issuance, so it builds and sends a guest link that `validate_guest_activity_token` rejects; recipients receive a link they cannot use.
Skip sending guest reminders when guest-token acceptance is disabled.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 5 findings to address first, and with guest-token issuance and acceptance enabled in the instance config, unauthenticated users may obtain and use guest activity tokens; if that trust boundary is too permissive, data or workflow access could be exposed before the configuration is reverted. The REST permission changes restrict writes, but reverting does not undo any access already granted.
Blocking findings: modules/weko-deposit/weko_deposit/config.py:131, modules/weko-workflow/weko_workflow/utils.py:3564, modules/invenio-deposit/invenio_deposit/views/rest.py:144, modules/weko-admin/weko_admin/utils.py:1997, modules/weko-admin/weko_admin/utils.py:1996
| # read_permission_factory_imp='', | ||
| # update_permission_factory_imp='', | ||
| delete_permission_factory_imp=deny_all, | ||
| create_decorators=['weko_accounts.utils:login_required_customize'], |
There was a problem hiding this comment.
🟠 High · Guests can create deposits
When an unauthenticated visitor has a guest-token session and the create permission factory does not reject that visitor, login_required_customize treats the guest-token session as authenticated, so the new create decorator lets the request reach deposit creation without an authenticated account or an item-specific create check. The guest can create a deposit.
Require an authenticated account for deposit creation or add a create permission check that rejects guest-token sessions.
Prompt for AI agents
In `modules/weko-deposit/weko_deposit/config.py` at line 131:
**Guests can create deposits**
When an unauthenticated visitor has a guest-token session and the create permission factory does not reject that visitor, `login_required_customize` treats the guest-token session as authenticated, so the new create decorator lets the request reach deposit creation without an authenticated account or an item-specific create check. The guest can create a deposit.
Require an authenticated account for deposit creation or add a create permission check that rejects guest-token sessions.| bool: True if ``WEKO_WORKFLOW_GUEST_TOKEN_ISSUE_ENABLED`` is set. | ||
| """ | ||
| return bool(current_app.config.get( | ||
| 'WEKO_WORKFLOW_GUEST_TOKEN_ISSUE_ENABLED', False)) |
There was a problem hiding this comment.
🟠 High · String false enables guest tokens
When an environment-specific config supplies either guest-token flag as the nonempty string "False", bool() treats any nonempty string as true, so a setting supplied as the string "False" leaves the corresponding guest-token gate enabled. Both issuing and accepting tokens can therefore remain enabled when an operator intended to disable them.
Require actual boolean config values or parse supported string values explicitly and reject malformed values.
Also at modules/weko-workflow/weko_workflow/utils.py:3574.
Prompt for AI agents
In `modules/weko-workflow/weko_workflow/utils.py` at line 3564:
**String false enables guest tokens**
When an environment-specific config supplies either guest-token flag as the nonempty string `"False"`, `bool()` treats any nonempty string as true, so a setting supplied as the string `"False"` leaves the corresponding guest-token gate enabled. Both issuing and accepting tokens can therefore remain enabled when an operator intended to disable them.
Require actual boolean config values or parse supported string values explicitly and reject malformed values.
Also at `modules/weko-workflow/weko_workflow/utils.py:3574`.| route_decorators = { | ||
| options['list_route']: ( | ||
| options.pop('create_decorators', []), ('POST',)), | ||
| options['item_route']: ( | ||
| options.pop('update_decorators', []), ('PUT', 'PATCH')), | ||
| } |
There was a problem hiding this comment.
🟠 High · File mutations skip edit checks
When an authenticated API caller has the write scope but lacks permission to edit the deposit, create_blueprint attaches update_decorators only to the item route, while file routes are registered separately; a caller with the API write scope but no item edit permission can therefore add, rename, reorder, or delete deposit files.
Apply the item edit check to file mutation routes too, or configure an update permission factory that enforces per-record edit access there.
Also at modules/weko-deposit/weko_deposit/config.py:132.
Prompt for AI agents
In `modules/invenio-deposit/invenio_deposit/views/rest.py` at lines 139-144:
**File mutations skip edit checks**
When an authenticated API caller has the write scope but lacks permission to edit the deposit, `create_blueprint` attaches `update_decorators` only to the item route, while file routes are registered separately; a caller with the API write scope but no item edit permission can therefore add, rename, reorder, or delete deposit files.
Apply the item edit check to file mutation routes too, or configure an update permission factory that enforces per-record edit access there.
Also at `modules/weko-deposit/weko_deposit/config.py:132`.| if (activity.extra_info or {}).get('is_guest') \ | ||
| and not self.__is_guest_token_issue_enabled(): | ||
| continue |
There was a problem hiding this comment.
🟡 Medium · Skipped guests trigger other reminder emails
When the activities contain a registered-user activity followed by a guest activity while guest token issuing is disabled, send_reminder_mail skips a disabled guest activity but leaves it in activity; the later guest check at line 2027 still runs and sends every accumulated entry in self.__mail_info_lst, including registered-user reminders that would not be sent without forced_send. The caller can therefore trigger unintended reminder emails.
Track whether any eligible guest activity was processed, and only use that state to trigger guest reminder sending.
Prompt for AI agents
In `modules/weko-admin/weko_admin/utils.py` at lines 1995-1997:
**Skipped guests trigger other reminder emails**
When the activities contain a registered-user activity followed by a guest activity while guest token issuing is disabled, `send_reminder_mail` skips a disabled guest activity but leaves it in `activity`; the later guest check at line 2027 still runs and sends every accumulated entry in `self.__mail_info_lst`, including registered-user reminders that would not be sent without `forced_send`. The caller can therefore trigger unintended reminder emails.
Track whether any eligible guest activity was processed, and only use that state to trigger guest reminder sending.| if (activity.extra_info or {}).get('is_guest') \ | ||
| and not self.__is_guest_token_issue_enabled(): |
There was a problem hiding this comment.
🟡 Medium · Reminders send unusable links
When guest token issuance is enabled, acceptance is disabled, and a guest activity is due for a reminder, UsageReport.send_reminder_mail checks only issuance, so it builds and sends a guest link that validate_guest_activity_token rejects; recipients receive a link they cannot use.
Skip sending guest reminders when guest-token acceptance is disabled.
Prompt for AI agents
In `modules/weko-admin/weko_admin/utils.py` at lines 1995-1996:
**Reminders send unusable links**
When guest token issuance is enabled, acceptance is disabled, and a guest activity is due for a reminder, `UsageReport.send_reminder_mail` checks only issuance, so it builds and sends a guest link that `validate_guest_activity_token` rejects; recipients receive a link they cannot use.
Skip sending guest reminders when guest-token acceptance is disabled.
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 2)サマリ
[WARN] W6 依存パッケージの版が変化した — 6件
[WARN] W5 監視対象 config が変化した — 1件
台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
|
概要 (Summary)
関連Issue / チケット (Related Issues)
変更タイプ (Type of Change)
🤖 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§3aSecret (
API_INVENTORY_REPO/API_INVENTORY_SSH_KEY) が未設定のリポジトリ、および fork からの PR では、このジョブは何もせずスキップされる。
API を追加・変更した場合（必須）
この 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)reconcile.md)*_PERMISSION_FACTORY/ CSRF 保護 等が危険側の値に変わったcan_delete/can_exportがFalse→Truedata_opを更新data_opが作成/更新/削除の経路に、未認証で到達したdata_opの記載誤りなら台帳を直すurl_mapに無いreconcile B のうち、実機に存在しないことが正当な行（プラグイン未登録・config で無効等）は
プライベートリポジトリの
reconcile_allow.jsonに理由付きで登録する。理由なしの登録は不可。登録済みの行は B'（既知・許容）として集計され、E'（endpoint が実機に無い）と併せてゲート対象外になる。
W1〜W6 は WARN でゲートは通るが、レビューでは見ること
（ModelView の追加 / 実装本体の変化 / HTTP メソッド・URL の変化 / 監視対象 config の変化 /
依存パッケージの版の変化）。特に W6（依存の版）は、ベースラインを CI と異なる環境で作ると
毎回出続けて形骸化するため、ベースラインは
install.shで作った環境から生成する。🔒 1. セキュリティ & API アクセス制御チェック (必須)
認証・認可 (Authentication & Authorization)
@login_required,@pass_record,need(...), Invenio Access Action/api/*ではPermission.require(http_exception=403)を使うこと。@login_requiredは API アプリにsecurity.loginが無いため 401 ではなく 500 になる--allow-writes付きでGET / HEAD 以外も叩く）
起動した経路のみ。ワークフロー系など未解決プレースホルダの行は skip される
Noneで無効化していない*_PERMISSION_FACTORY等を監視機能クローズ・非公開化の場合 (Feature Disable)
404 Not Foundまたは403 Forbiddenが返ることを確認した🧪 2. テストコード観点チェック (pytest / Invenio Test Suite)
権限・異常系テスト (Negative & Authorization Tests)
401 Unauthorizedまたは403 Forbidden/404 Not Foundが返ることを検証するテストがある403になるテストがある404/403を返すテストがある境界値・入力バリデーションテスト (Boundary & Validation)
400 Bad Request/ バリデーションエラーが返るテストがあるデータ整合性・トランザクションテスト (Integrity & Rollback)
🛡️ 3. データ保護 & 破壊的変更防止チェック (Data Safety)
⚙️ 4. マイグレーション & システム影響チェック (Invenio / WEKO3 Stack)
データベース (DB / Alembic)
invenio alembic upgrade（適用）およびdowngrade（ロールバック）スクリプトを作成・検証した検索インデックス (Elasticsearch / OpenSearch)
設定 & 非同期処理 (Config / Celery / Cache)
invenio.cfg/ 環境変数のデフォルト値を設定した📚 5. ドキュメント・仕様書更新チェック (weko-document)
tools/api-inventory/、台帳・調査記録はプライベートリポジトリ（public リポジトリには置かない）。
§0 のチェック項目で対応済みなら、ここは確認のみ。
weko3_api_auth_findings.md）もプライベートリポジトリに置く。台帳は二重管理しない📋 6. 動作検証エビデンス (Verification Evidence)
テスト実行結果
CI の成果物 (artifact:
api-inventory-summary)drift.mdreconcile.md明細（該当した経路名・実測結果）は公開できないため artifact に含めていない。
プライベートリポジトリ側で同じコマンドを
--summary-onlyなしで実行して確認する。手動で確認したこと
Summary by Sourcery
Harden guest activity access and REST mutation authorization while adding configuration-based controls for guest token functionality.
New Features:
Bug Fixes:
Enhancements:
Tests: