Hotfix/issue62807

.github/workflows/api-inventory-drift.yml

@@ -106,6 +106,18 @@ jobs:
         with:
           python-version: '3.11'
 
+      # ソース(AST)と台帳だけで済むので、コンテナを立てる前に流す。
+      # ビュー関数が HTTP を通らず別モジュールから直接呼ばれる「第二の入口」は
+      # ルート単位の台帳に現れず、認可を足す作業で素通りする(issue62807)。
+      # 台帳の inproc_callers に無い呼び出し元が現れたら止める。
+      - name: Check in-process view callers
+        if: steps.cfg.outputs.enabled == 'true'
+        env:
+          WEKO_API_INVENTORY_DIR: ${{ github.workspace }}/.api-inventory-data
+        run: |
+          python3 tools/api-inventory/scripts/add_inproc_callers.py \
+            --check --summary-only --gate
+
       - name: Start WEKO containers
         if: steps.cfg.outputs.enabled == 'true'
         run: |

modules/weko-records-ui/tests/test_permissions.py

@@ -923,6 +923,76 @@ def test_check_created_id_proxy_posting(app, users, proxy_posting, position,
         app.config["WEKO_ITEMS_UI_PROXY_POSTING"] = original
 
 
+# .tox/c1/bin/pytest --cov=weko_records_ui tests/test_permissions.py::test_record_edit_permission_required_id_sources -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-records-ui/.tox/c1/tmp
+@pytest.mark.parametrize("call_kwargs,ctx_kwargs,expected", [
+    # URL ルート経由 (/records/soft_delete/<recid>) は kwargs で届く
+    ({"kwargs": {"recid": "1"}}, {}, "1"),
+    # Python から位置引数で直接呼ぶ経路 (prepare_delete_item など)。
+    # ボディのキーは pid_value なので、位置引数から拾えないと 400 になる
+    ({"args": ("1",)}, {"json": {"pid_value": "1"}}, "1"),
+    # 位置引数の del_ver_ も剥がしてから引く
+    ({"args": ("del_ver_1",)}, {"json": {"pid_value": "1"}}, "1"),
+    # フォーム経由 (replace_file / get_file_place)
+    ({}, {"data": {"recid": "1"}}, "1"),
+    # JSON ボディ経由 (copy_bucket)
+    ({}, {"json": {"recid": "1"}}, "1"),
+    # クエリ文字列経由
+    ({}, {"query_string": {"recid": "1"}}, "1"),
+])
+def test_record_edit_permission_required_id_sources(
+        app, users, call_kwargs, ctx_kwargs, expected):
+    """recid の解決元。位置引数を落とすと画面からの削除が全部 400 になる。"""
+    from weko_records_ui.permissions import record_edit_permission_required
+
+    seen = []
+
+    @record_edit_permission_required(strip_prefix="del_ver_")
+    def view(recid=None):
+        seen.append(recid)
+        return "ok"
+
+    with patch("flask_login.utils._get_user", return_value=users[2]["obj"]):
+        with patch("weko_records_ui.permissions.check_created_id_by_recid",
+                   return_value=True) as mock_check:
+            with app.test_request_context("/", method="POST", **ctx_kwargs):
+                assert view(*call_kwargs.get("args", ()),
+                            **call_kwargs.get("kwargs", {})) == "ok"
+
+    # 権限判定には prefix を剥がした id を渡す
+    mock_check.assert_called_once_with(expected)
+    # ビュー本体には受け取ったままの値を渡す (剥がすのはビューの仕事)
+    assert len(seen) == 1
+
+
+# .tox/c1/bin/pytest --cov=weko_records_ui tests/test_permissions.py::test_record_edit_permission_required_aborts -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-records-ui/.tox/c1/tmp
+@pytest.mark.parametrize("authenticated,permitted,recid,expected_code", [
+    (False, True, "1", 401),    # 未認証
+    (True, True, None, 400),    # どこにも id が無い
+    (True, False, "1", 403),    # 権限なし
+])
+def test_record_edit_permission_required_aborts(
+        app, users, authenticated, permitted, recid, expected_code):
+    """id が取れないときだけ 400。権限で弾くのは 403、未認証は 401。"""
+    from werkzeug.exceptions import HTTPException
+    from weko_records_ui.permissions import record_edit_permission_required
+
+    @record_edit_permission_required()
+    def view(recid=None):
+        return "ok"
+
+    user = users[2]["obj"] if authenticated else None
+    args = (recid,) if recid is not None else ()
+
+    with patch("flask_login.utils._get_user", return_value=user):
+        with patch("weko_records_ui.permissions.check_created_id_by_recid",
+                   return_value=permitted):
+            with app.test_request_context("/", method="POST"):
+                with pytest.raises(HTTPException) as exc:
+                    view(*args)
+
+    assert exc.value.code == expected_code
+
+
 # .tox/c1/bin/pytest --cov=weko_records_ui tests/test_permissions.py::test_check_created_id -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-records-ui/.tox/c1/tmp
 @pytest.mark.parametrize("index,status",[
     (0,False),

modules/weko-records-ui/tests/test_views.py

@@ -1430,6 +1430,52 @@ def test_soft_delete_exception(client, records, users):
                     assert res.json == expected_response
 
 
+# .tox/c1/bin/pytest --cov=weko_records_ui tests/test_views.py::test_soft_delete_called_in_process -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-records-ui/.tox/c1/tmp
+@pytest.mark.parametrize(
+    "del_value, expected_recid",
+    [
+        ("1", "1"),                # 通常の削除
+        ("del_ver_1", "1"),        # バージョン削除
+    ],
+)
+def test_soft_delete_called_in_process(app, records, users, del_value,
+                                       expected_recid):
+    """soft_delete ビューを Python から位置引数で呼ぶ経路を守る。
+
+    画面の削除ボタンは POST /items/prepare_delete_item に
+    {"pid_value": ...} を投げ、weko_items_ui.views.prepare_delete_item が
+    ``from weko_records_ui.views import soft_delete`` して
+    ``soft_delete(del_value)`` と *位置引数* で呼ぶ
+    (weko_workflow.utils.prepare_delete_workflow も同じ)。
+
+    このとき kwargs は空で、リクエストボディのキーも recid ではなく
+    pid_value なので、record_edit_permission_required が recid を
+    見つけられずに abort(400) していた (v2.0.4 の回帰)。
+    リクエストは views.py:1362 の try の外なので、素の 400 が返って
+    削除が誰も実行できなくなる。
+    """
+    from weko_records_ui.views import soft_delete
+
+    with patch("flask_login.utils._get_user", return_value=users[2]["obj"]):
+        with app.test_request_context(
+            "/items/prepare_delete_item",
+            method="POST",
+            json={"pid_value": expected_recid},
+        ):
+            with patch("weko_records_ui.views.soft_delete_imp") as mock_imp, \
+                 patch("weko_records_ui.views.delete_version") as mock_ver, \
+                 patch("weko_records_ui.views.call_external_system"):
+                res = soft_delete(del_value)
+
+            assert res.status_code == 200
+            if del_value.startswith("del_ver_"):
+                mock_ver.assert_called_once_with(expected_recid)
+                mock_imp.assert_not_called()
+            else:
+                mock_imp.assert_called_once_with(expected_recid)
+                mock_ver.assert_not_called()
+
+
 # def restore(recid):
 # .tox/c1/bin/pytest --cov=weko_records_ui tests/test_views.py::test_restore_acl_guest -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-records-ui/.tox/c1/tmp
 def test_restore_acl_guest(client, records):

modules/weko-records-ui/weko_records_ui/permissions.py

@@ -23,6 +23,7 @@
 from datetime import datetime as dt
 from datetime import timedelta, timezone
 from functools import wraps
+import inspect
 import traceback
 from typing import List, Optional
 
@@ -528,9 +529,11 @@ def check_created_id_by_recid(recid):
 def record_edit_permission_required(param='recid', strip_prefix=None):
     """Require edit permission on the record identified by ``param``.
 
-    The record id is resolved from the view args first, then the request body
-    (form or JSON) and finally the query string, so the same decorator covers
-    ``/records/soft_delete/<recid>`` and POSTs that carry ``pid`` in the form.
+    The record id is resolved from the view args first (keyword *and*
+    positional, so that in-process calls to the view keep working), then the
+    request body (form or JSON) and finally the query string, so the same
+    decorator covers ``/records/soft_delete/<recid>`` and POSTs that carry
+    ``pid`` in the form.
 
     The check itself is :func:`check_created_id`: the creator, a shared user,
     a Community Administrator of the record's community, or a super user.
@@ -553,6 +556,18 @@ def decorated(*args, **kwargs):
                 abort(401)
 
             recid = kwargs.get(param)
+            if recid is None and args:
+                # ビュー関数を HTTP 経由ではなく Python から直接呼ぶ経路が
+                # ある (weko_items_ui.views.prepare_delete_item と
+                # weko_workflow.utils.prepare_delete_workflow が
+                # soft_delete(del_value) と位置引数で呼ぶ)。
+                # そこでは kwargs もリクエストボディも param を持たないため、
+                # シグネチャに束ねて位置引数からも取り出す。
+                try:
+                    bound = inspect.signature(f).bind_partial(*args, **kwargs)
+                    recid = bound.arguments.get(param)
+                except TypeError as e:
+                    current_app.logger.error(e)
             if recid is None:
                 recid = request.form.get(param)
             if recid is None and request.mimetype == 'application/json':

tools/api-inventory/ci/api-inventory-drift.yml

@@ -113,6 +113,18 @@ jobs:
         with:
           python-version: '3.11'
 
+      # ソース(AST)と台帳だけで済むので、コンテナを立てる前に流す。
+      # ビュー関数が HTTP を通らず別モジュールから直接呼ばれる「第二の入口」は
+      # ルート単位の台帳に現れず、認可を足す作業で素通りする(issue62807)。
+      # 台帳の inproc_callers に無い呼び出し元が現れたら止める。
+      - name: Check in-process view callers
+        if: steps.cfg.outputs.enabled == 'true'
+        env:
+          WEKO_API_INVENTORY_DIR: ${{ github.workspace }}/.api-inventory-data
+        run: |
+          python3 tools/api-inventory/scripts/add_inproc_callers.py \
+            --check --summary-only --gate
+
       - name: Start WEKO containers
         if: steps.cfg.outputs.enabled == 'true'
         run: |

tools/api-inventory/scripts/README.md

@@ -59,11 +59,44 @@ cd /path/to/weko          # ツールは WEKO3 リポジトリ側にある
 判定ルールを変えた、テストを追加した、といったとき。
 
 ```bash
+python3 tools/api-inventory/scripts/add_inproc_callers.py  # in-process 呼び出し元を付与
 python3 tools/api-inventory/scripts/test_coverage.py    # テスト4観点を判定
 python3 tools/api-inventory/scripts/prioritize.py       # 優先度・整理対象を付与
 python3 tools/api-inventory/scripts/build_checklist.py  # 32列版を再生成
 ```
 
+## ★認可を足す前に in-process の呼び出し元を見る (issue62807)
+
+台帳は Flask のルートを単位にしている。だが WEKO3 には、ビュー関数を
+HTTP を通さず別モジュールから直接呼ぶ「第二の入口」がある。
+この入口は台帳のどの列にも現れないため、**ルート単位で認可を足していく
+作業では素通りする**。
+
+issue62807 がその実例。v2.0.4 で `soft_delete` に
+`record_edit_permission_required` を足したが、このデコレータは recid を
+kwargs / request.form / JSON body / query string からしか探していなかった。
+画面の削除は `POST /items/prepare_delete_item` に `{"pid_value": ...}` を投げ、
+`weko_items_ui.views.prepare_delete_item` が
+``soft_delete(del_value)`` と **位置引数** でビューを直接呼ぶ。
+kwargs は空、ボディのキーも recid ではないので id が取れず abort(400)。
+権限判定より前で落ちるため、作成者でも管理者でも削除できなくなった。
+
+台帳上この行は `auth_required=要` / `test_gap=-` で、穴が無いように見えていた。
+`inproc_callers` 列はこの死角を可視化するために足した。
+
+```bash
+# 単体で確認する(列を書かずに一覧だけ見る)
+WEKO_ROOT=/home/mhaya/wekov2 python3 tools/api-inventory/scripts/audit_inprocess_views.py
+
+# CI から回すときは件数だけ(ログ・artifact・PRコメントは誰でも読める)
+python3 .../audit_inprocess_views.py --summary-only --fail-on-high
+```
+
+`risk=HIGH` は「認可デコレータ付きのビューを、位置引数で in-process 呼び出し
+している」もの。デコレータは呼び出し元のリクエストコンテキストで動くので、
+**リクエストから値を読むデコレータをこの種のビューに付けてはいけない**。
+付けるなら、位置引数からも値を解決できることを確かめる。
+
 ## ケース2: 台帳に行を追加する
 
 `reconcile.py` が「A. インベントリ未収載」を出したとき。62列を手で並べる必要はない。
@@ -612,6 +645,8 @@ git push origin main --follow-tags
 | `apply2.py` / `check_reachable.py` / `dump_modelviews.py` | — | Phase 1-3 の使い捨て。パスが決め打ちなので、そのままでは回らない。参考として残してある |
 | `remeasure.sh` | — | 非推奨。`measure.sh` に統合(案内のみ) |
 | `add_cols.py` / `add_ssrf_redirect.py` / `add_idempotency.py` / `add_dataop4.py` / `add_authmech.py` | full.tsv + 実装ソース | full.tsv の**空欄/TODO セルのみ**を機械付与 |
+| `audit_inprocess_views.py` | 実装ソース(AST) | 何も書かない(in-process 呼び出しを報告するだけ) |
+| `add_inproc_callers.py` | full.tsv + `audit_inprocess_views.py` | full.tsv の `inproc_callers`(**空欄/TODO セルのみ**) |
 
 `test_coverage.py` → `prioritize.py` → `build_checklist.py` は**何度流しても結果が変わらない**
 (冪等)。32列版は full.tsv から完全に再現できることを確認済み。