From e3ab9af17c42907e771f330a138d02b75677fb06 Mon Sep 17 00:00:00 2001 From: Masaharu Hayashi Date: Mon, 7 Sep 2026 01:00:03 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(records-ui):=20=E8=AA=8D=E5=8F=AF?= =?UTF-8?q?=E3=83=87=E3=82=B3=E3=83=AC=E3=83=BC=E3=82=BF=E3=81=8C=E4=BD=8D?= =?UTF-8?q?=E7=BD=AE=E5=BC=95=E6=95=B0=E3=81=AE=20recid=20=E3=82=92?= =?UTF-8?q?=E8=A6=8B=E3=81=A6=E3=81=84=E3=81=AA=E3=81=84=20(issue62807)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v2.0.4 (2f6b61b2f) で 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) と *位置引数* で直接呼ぶ (weko_workflow.utils.prepare_delete_workflow も同じ)。 このとき kwargs は空、ボディのキーも recid ではなく pid_value なので、 recid が見つからず abort(400) していた。判定は権限チェックより前なので、 作成者でも管理者でも一律にアイテムを削除できない。 views.py:1362 の呼び出しは try の外にあり、JSON ではなく素の 400 が返る。 inspect.signature().bind_partial() でシグネチャに束ね、位置引数からも recid を解決するようにした。in-process 呼び出しがあるのは soft_delete だけで、 restore / copy_bucket / get_file_place / replace_file には無い。 テストは本番の呼び出し形をそのまま再現する結合テスト (test_views.py) と、 recid の解決元 6 パターン + 401/400/403 を見る単体テスト (test_permissions.py) を追加した。修正前は位置引数の 2 ケースと 403 ケースが 400 に化けて落ちる。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NJUSCriemeQBj1WU7ttXV1 --- .../weko-records-ui/tests/test_permissions.py | 70 +++++++++++++++++++ modules/weko-records-ui/tests/test_views.py | 46 ++++++++++++ .../weko_records_ui/permissions.py | 21 +++++- 3 files changed, 134 insertions(+), 3 deletions(-) diff --git a/modules/weko-records-ui/tests/test_permissions.py b/modules/weko-records-ui/tests/test_permissions.py index 6a513bf8d1..a69b856e94 100644 --- a/modules/weko-records-ui/tests/test_permissions.py +++ b/modules/weko-records-ui/tests/test_permissions.py @@ -892,6 +892,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/) は 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), diff --git a/modules/weko-records-ui/tests/test_views.py b/modules/weko-records-ui/tests/test_views.py index 3384266728..df6d44de93 100644 --- a/modules/weko-records-ui/tests/test_views.py +++ b/modules/weko-records-ui/tests/test_views.py @@ -1372,6 +1372,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): diff --git a/modules/weko-records-ui/weko_records_ui/permissions.py b/modules/weko-records-ui/weko_records_ui/permissions.py index fe78ae7a73..22c6d62a97 100644 --- a/modules/weko-records-ui/weko_records_ui/permissions.py +++ b/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 @@ -524,9 +525,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/`` 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/`` 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. @@ -549,6 +552,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': From 4039b4cb1e0bf832b7ba76a28ae721c3b97b3b7e Mon Sep 17 00:00:00 2001 From: Masaharu Hayashi Date: Mon, 7 Sep 2026 01:23:53 +0000 Subject: [PATCH 2/2] =?UTF-8?q?feat(api-inventory):=20=E3=83=93=E3=83=A5?= =?UTF-8?q?=E3=83=BC=E3=81=AE=20in-process=20=E5=91=BC=E3=81=B3=E5=87=BA?= =?UTF-8?q?=E3=81=97=E3=82=92=E5=8F=B0=E5=B8=B3=E3=81=AB=E8=BC=89=E3=81=9B?= =?UTF-8?q?=E3=82=8B=20(issue62807)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 台帳は extract_routes.py が拾う Flask のルートを単位にしている。だが WEKO3 には ビュー関数を HTTP を通さず別モジュールから直接呼ぶ「第二の入口」があり、 これは台帳のどの列にも現れない。ルート単位で認可を足していく作業では素通りする。 issue62807 がその実例。soft_delete の行は auth_required=要 / test_gap=- で 穴が無いように見えていたが、実際には prepare_delete_item が soft_delete(del_value) と位置引数で直接呼んでおり、v2.0.4 で足した record_edit_permission_required が recid を見つけられず abort(400) していた。 - audit_inprocess_views.py: ルート登録されたビューのうち、本体コードから 名前で import されているものを AST で列挙する。認可デコレータ付きを 位置引数で呼んでいるものは risk=HIGH。 - add_inproc_callers.py: 台帳に inproc_callers 列を付与する (62→63列)。 --check は書き込まず、台帳がまだ知らない呼び出し元だけを報告する。 - api-inventory-drift.yml: --check --summary-only --gate を CI に入れた。 ソースと台帳だけで済むのでコンテナ起動の前に流す。 - README: ケース1 の手順、認可を足す前の確認、スクリプトの入出力表を更新。 現状の検出結果は HIGH 5 件。うち 2 件が issue62807 (soft_delete)、 残り 3 件は HeadlessActivity が prepare_edit_item / prepare_delete_item / check_validation_error_msg を位置引数で呼ぶもの (v2.0.4 以前から存在)。 後者のデコレータはリクエストから値を読まないため同じ失敗はしないが、 prepare_delete_item 経由で issue62807 の影響は受ける。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NJUSCriemeQBj1WU7ttXV1 --- .github/workflows/api-inventory-drift.yml | 12 + .../api-inventory/ci/api-inventory-drift.yml | 12 + tools/api-inventory/scripts/README.md | 35 +++ .../scripts/add_inproc_callers.py | 184 ++++++++++++ .../scripts/audit_inprocess_views.py | 277 ++++++++++++++++++ 5 files changed, 520 insertions(+) create mode 100644 tools/api-inventory/scripts/add_inproc_callers.py create mode 100644 tools/api-inventory/scripts/audit_inprocess_views.py diff --git a/.github/workflows/api-inventory-drift.yml b/.github/workflows/api-inventory-drift.yml index f21290378a..90abf5e6e3 100644 --- a/.github/workflows/api-inventory-drift.yml +++ b/.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: | diff --git a/tools/api-inventory/ci/api-inventory-drift.yml b/tools/api-inventory/ci/api-inventory-drift.yml index f21290378a..90abf5e6e3 100644 --- a/tools/api-inventory/ci/api-inventory-drift.yml +++ b/tools/api-inventory/ci/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: | diff --git a/tools/api-inventory/scripts/README.md b/tools/api-inventory/scripts/README.md index 91ea82f8c7..68f5aca40a 100644 --- a/tools/api-inventory/scripts/README.md +++ b/tools/api-inventory/scripts/README.md @@ -52,11 +52,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 # 24列版を再生成 ``` +## ★認可を足す前に 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. インベントリ未収載」を出したとき。57列を手で並べる必要はない。 @@ -545,6 +578,8 @@ git push origin main --follow-tags | `_ensure_profile.py` / `_read_profile.py` / `_targets.py` / `_report.py` | — | `measure.sh` の内部ヘルパ | | `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` は**何度流しても結果が変わらない** (冪等)。24列版は full.tsv から完全に再現できることを確認済み。 diff --git a/tools/api-inventory/scripts/add_inproc_callers.py b/tools/api-inventory/scripts/add_inproc_callers.py new file mode 100644 index 0000000000..fc3f431693 --- /dev/null +++ b/tools/api-inventory/scripts/add_inproc_callers.py @@ -0,0 +1,184 @@ +# -*- coding: utf-8 -*- +"""台帳に `inproc_callers` 列(in-process からの呼び出し元)を付与する。 + +## なぜ要るのか + +台帳は `extract_routes.py` が拾う Flask のルートを単位にしている。 +そのため「ビュー関数が HTTP を通らず、別モジュールから直接呼ばれる」 +第二の入口はどの列にも現れない。ルート単位で認可を足していく作業では、 +この入口が視界に入らないまま素通りする。 + +issue62807 はその実例。`soft_delete` の行は auth_required=要 / +test_gap=- で穴が無いように見えていたが、実際には +`weko_items_ui.views.prepare_delete_item` が +``soft_delete(del_value)`` と位置引数で直接呼んでおり、 +v2.0.4 で足した `record_edit_permission_required` が recid を +見つけられず abort(400) して、全ユーザがアイテムを削除できなくなった。 + +この列があれば「認可を足す前に、HTTP 以外の呼び出し元を確認する」 +判断ができる。 + +## 値の形式 + + - in-process からの呼び出しなし + 位置引数::;kwargs:: + 呼び出し元を列挙する。位置引数のものを先に置く + (kwargs しか見ないデコレータが壊れるのは位置引数の側)。 + +## 使い方 + + export WEKO_API_INVENTORY_DIR=/path/to/weko-secret + WEKO_ROOT=/home/mhaya/wekov2 python3 add_inproc_callers.py + +既存値は上書きしない(空欄/TODO のみ埋める)。判定を入れ直したいときは +``WEKO_INVENTORY_OVERWRITE=1`` を付ける。add_cols.py と同じ規約。 + +CI では台帳に書かず、**台帳がまだ知らない呼び出し元**だけを見る: + + python3 add_inproc_callers.py --check --summary-only --gate + +認可を足す PR でこの件数が増えたら、そのビューには HTTP 以外の入口がある。 +デコレータがそちらでも成立するかを確かめてからマージすること。 +``--summary-only`` は件数だけを出す (CI のログ・artifact・PR コメントは +誰でも読めるため。他のスクリプトと同じ方針)。 +""" +import collections +import os as _os +import sys as _sys + +_sys.path.insert(0, _os.path.dirname(_os.path.abspath(__file__))) +from paths import data_path as _data_path # noqa: E402 +from audit_inprocess_views import ( # noqa: E402 + collect_imports, collect_views, positional_calls, +) + +# 既定は $WEKO_API_INVENTORY_DIR/weko3_api_list_full.tsv。 +# 位置引数で別のパスを渡せる (argparse 側の default)。 +TSV = _data_path("weko3_api_list_full.tsv") + +COL = "inproc_callers" + + +def load(p): + return [l.rstrip("\n").split("\t") + for l in open(p, encoding="utf-8") if l.rstrip("\n")] + + +def _write(path, hd, data, newcols): + """add_cols.py の _write と同じ規約。 + + 既存の同名列はその位置のまま値を差し替え、無い列だけ末尾に足す。 + 既存値は人手で精査されているので、空欄/TODO のセルだけ埋める。 + """ + pos = {n: i for i, n in enumerate(hd)} + out_hd = list(hd) + [n for n in newcols if n not in pos] + force = _os.environ.get("WEKO_INVENTORY_OVERWRITE") == "1" + empty = ("", "-", "TODO") + lines = ["\t".join(out_hd)] + filled = 0 + for c in data: + row = list(c) + [""] * (len(hd) - len(c)) + vals = row[len(hd):] + body = row[:len(hd)] + for n, v in zip(newcols, vals): + if n not in pos: + continue + cur = body[pos[n]] + if cur in empty or force: + if cur != v: + filled += 1 + body[pos[n]] = v + extra = [v for n, v in zip(newcols, vals) if n not in pos] + lines.append("\t".join(str(x).replace("\t", " ") + for x in body + extra)) + open(path, "w", encoding="utf-8").write("\n".join(lines) + "\n") + print(" → 空欄/TODO を埋めたセル: {}{}".format( + filled, + " (WEKO_INVENTORY_OVERWRITE=1 のため既存値も上書き)" if force else "")) + + +def build_index(): + """{(impl_file, impl_func): [呼び出し元の説明文字列]}""" + views = collect_views() + idx = collections.defaultdict(list) + for mod, name, importer, line in collect_imports(): + v = views.get((mod, name)) + if v is None or importer == v["file"]: + continue + pos_lines = positional_calls(importer, {name}).get(name, []) + if pos_lines: + for ln in pos_lines: + idx[(v["file"], v["func"])].append( + "位置引数:{}:{}".format(importer, ln)) + else: + idx[(v["file"], v["func"])].append( + "kwargs:{}:{}".format(importer, line)) + # 位置引数のものを先に、あとは安定順で + for k in idx: + idx[k] = sorted(set(idx[k]), + key=lambda s: (not s.startswith("位置引数"), s)) + return idx + + +def main(): + import argparse + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("tsv", nargs="?", default=TSV) + ap.add_argument("--check", action="store_true", + help="台帳に書かず、ソースと台帳のズレだけを報告する(CI 用)") + ap.add_argument("--summary-only", action="store_true", + help="件数だけ出す (CI 用。ログが公開されるため)") + ap.add_argument("--gate", action="store_true", + help="台帳に無い in-process 呼び出しがあれば終了コード1") + args = ap.parse_args() + + rows = load(args.tsv) + hd, data = rows[0], rows[1:] + cache = {n: i for i, n in enumerate(hd)} + + def col(c, name): + i = cache.get(name) + return c[i] if i is not None and len(c) > i else "" + + idx = build_index() + + if args.check: + # 台帳が把握していない in-process 呼び出しを洗う。 + # 認可を足す PR でこれが増えたら、そのビューには第二の入口がある。 + drift = [] + for c in data: + found = set(idx.get((col(c, "impl_file"), col(c, "impl_func")), [])) + recorded = set(x for x in col(c, COL).split(";") if x and x != "-") + new_callers = sorted(found - recorded) + if new_callers: + drift.append((col(c, "method"), col(c, "uri"), new_callers)) + if args.summary_only: + print("台帳に無い in-process 呼び出し: {} 件".format(len(drift))) + else: + for m, uri, callers in drift: + print("[新規] {} {} ← {}".format(m, uri, ";".join(callers))) + print("\n台帳に無い in-process 呼び出し: {} 件".format(len(drift))) + return 1 if (args.gate and drift) else 0 + + for c in data: + c.append(";".join(idx.get((col(c, "impl_file"), + col(c, "impl_func")), [])) or "-") + + _write(args.tsv, hd, data, [COL]) + + ci = cache.get(COL, len(hd)) + hits = [c for c in data if len(c) > ci and c[ci] not in ("", "-")] + if args.summary_only: + print("{}: 呼び出し元あり={} 件 / 全 {} 行".format( + COL, len(hits), len(data))) + else: + print("{}: 呼び出し元あり={} 件 / 全 {} 行".format( + COL, len(hits), len(data))) + for c in hits: + print(" {} {} ← {}".format(col(c, "method"), col(c, "uri"), c[ci])) + print("列数:", len(hd) + (0 if COL in cache else 1)) + return 0 + + +if __name__ == "__main__": + _sys.exit(main()) diff --git a/tools/api-inventory/scripts/audit_inprocess_views.py b/tools/api-inventory/scripts/audit_inprocess_views.py new file mode 100644 index 0000000000..92b4fca489 --- /dev/null +++ b/tools/api-inventory/scripts/audit_inprocess_views.py @@ -0,0 +1,277 @@ +# -*- coding: utf-8 -*- +"""ビュー関数が in-process からも呼ばれている経路を検出する (AST)。 + +## なぜ要るのか + +台帳は `extract_routes.py` が拾う **Flask のルート** を単位にしている。 +だが WEKO3 には、ビュー関数を HTTP 経由ではなく別モジュールから +``from weko_records_ui.views import soft_delete`` して直接呼ぶ経路がある。 +この「第二の入口」は台帳のどの列にも現れないため、ルート単位で認可を +足していく作業では見落とされる。 + +issue62807 がその実例: +``record_edit_permission_required`` は recid を kwargs / request.form / +JSON body / query string からしか探しておらず、``soft_delete(del_value)`` と +*位置引数* で呼ぶ in-process 経路で id を見つけられず abort(400) していた。 +台帳上この行は auth_required=要 / test_gap=- で、穴が無いように見えていた。 + +## 何を出すか + +ルート/expose/add_url_rule で登録されたビュー関数のうち、 +**テスト以外の本体コードから名前で import されている** ものを列挙する。 +デコレータが付いているものは、呼び出し元のリクエストコンテキストで +そのデコレータが動くことになるため risk=HIGH として区別する。 + +## 使い方 + + WEKO_ROOT=/home/mhaya/wekov2 python3 audit_inprocess_views.py + WEKO_ROOT=/home/mhaya/wekov2 python3 audit_inprocess_views.py --json out.json + WEKO_ROOT=/home/mhaya/wekov2 python3 audit_inprocess_views.py --summary-only + +``--summary-only`` は件数だけを出す。CI のログ・artifact・PR コメントは +誰でも読めるため、CI から回すときは必ずこちらを使うこと +(``tools/api-inventory/ci/README.md`` と同じ方針)。 +""" +import argparse +import ast +import json +import os +import sys + +def _find_root(): + """WEKO3 リポジトリのルート(`modules/` を持つ階層)を探す。 + + ツールは WEKO3 本体の tools/api-inventory/scripts/ に置かれることも、 + weko-document 側に置かれることもある(README 冒頭の注記)。 + 決め打ちの相対パスだと片方で外れるので、`modules/` の有無で決める。 + """ + env = os.environ.get('WEKO_ROOT') + if env: + return env + d = os.path.dirname(os.path.abspath(__file__)) + while True: + if os.path.isdir(os.path.join(d, 'modules')): + return d + parent = os.path.dirname(d) + if parent == d: + return os.getcwd() + d = parent + + +ROOT = _find_root() +SKIP = ('/tests', '/examples', '/.tox', '/node_modules', '/docs/', '/build/', + '/cookiecutter') + +# audit_decorators.py と同じ集合。認可デコレータの名寄せ。 +AUTH = { + 'login_required', 'login_required_customize', 'roles_required', + 'require_api_auth', 'require_oauth_scopes', 'need_record_permission', + 'need_permissions', 'check_authority', 'stats_api_access_required', + 'check_index_access_permissions', 'check_on_behalf_of', 'require_oauth', + 'pass_record', 'record_edit_permission_required', + 'require_item_edit_permission', +} + + +def iter_py(): + for dp, _dn, fn in os.walk(os.path.join(ROOT, 'modules')): + if any(s in dp + '/' for s in SKIP): + continue + for f in sorted(fn): + if f.endswith('.py'): + yield os.path.join(dp, f) + + +def dec_name(d): + c = d.func if isinstance(d, ast.Call) else d + parts = [] + while isinstance(c, ast.Attribute): + parts.append(c.attr) + c = c.value + if isinstance(c, ast.Name): + parts.append(c.id) + return '.'.join(reversed(parts)) + + +def is_auth(name): + last = name.split('.')[-1] + return last in AUTH or name.endswith('permission.require') + + +def mod_of(rel): + """modules/weko-records-ui/weko_records_ui/views.py -> weko_records_ui.views""" + parts = rel.split(os.sep) + if len(parts) < 3 or parts[0] != 'modules': + return None + return '.'.join(parts[2:])[:-3] if parts[-1].endswith('.py') else None + + +def collect_views(): + """{(module, func): {...}} ルート登録されたビュー関数。""" + views = {} + url_rule_funcs = set() # add_url_rule(view_func=...) で登録される名前 + + for fp in iter_py(): + rel = os.path.relpath(fp, ROOT) + mod = mod_of(rel) + if not mod: + continue + try: + tree = ast.parse(open(fp, encoding='utf-8', errors='replace').read()) + except Exception: + continue + + for node in ast.walk(tree): + # add_url_rule(..., view_func=publish) 形式 + if isinstance(node, ast.Call) and \ + dec_name(node).endswith('add_url_rule'): + for kw in node.keywords: + if kw.arg == 'view_func' and isinstance(kw.value, ast.Name): + url_rule_funcs.add((mod, kw.value.id)) + + if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + continue + decs = [dec_name(d) for d in node.decorator_list] + routed = any(d.endswith('.route') or d == 'expose' + or d.endswith('.expose') for d in decs) + if not routed: + continue + views[(mod, node.name)] = { + 'module': mod, + 'func': node.name, + 'file': rel, + 'line': node.lineno, + 'decorators': decs, + 'auth_decorators': [d for d in decs if is_auth(d)], + } + + # add_url_rule 経由も、関数定義が見つかればビューとして扱う + for fp in iter_py(): + rel = os.path.relpath(fp, ROOT) + mod = mod_of(rel) + if not mod: + continue + try: + tree = ast.parse(open(fp, encoding='utf-8', errors='replace').read()) + except Exception: + continue + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and \ + (mod, node.name) in url_rule_funcs and \ + (mod, node.name) not in views: + decs = [dec_name(d) for d in node.decorator_list] + views[(mod, node.name)] = { + 'module': mod, + 'func': node.name, + 'file': rel, + 'line': node.lineno, + 'decorators': decs, + 'auth_decorators': [d for d in decs if is_auth(d)], + 'registered_via': 'add_url_rule', + } + return views + + +def collect_imports(): + """[(module, name, importer_file, line)] 本体コードからの from-import。""" + found = [] + for fp in iter_py(): + rel = os.path.relpath(fp, ROOT) + try: + tree = ast.parse(open(fp, encoding='utf-8', errors='replace').read()) + except Exception: + continue + for node in ast.walk(tree): + if not isinstance(node, ast.ImportFrom) or not node.module: + continue + # 相対 import (level>0) は同一モジュール内なので対象外。 + # 見たいのは「別モジュールがビューを直接呼ぶ」ケース。 + if node.level: + continue + for a in node.names: + found.append((node.module, a.name, rel, node.lineno)) + return found + + +def positional_calls(rel_file, names): + """importer の中で、その名前を位置引数付きで呼んでいる行を返す。""" + hits = {} + fp = os.path.join(ROOT, rel_file) + try: + tree = ast.parse(open(fp, encoding='utf-8', errors='replace').read()) + except Exception: + return hits + for node in ast.walk(tree): + if isinstance(node, ast.Call) and isinstance(node.func, ast.Name) \ + and node.func.id in names and node.args: + hits.setdefault(node.func.id, []).append(node.lineno) + return hits + + +def main(): + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument('--json', help='結果を JSON で書き出す') + ap.add_argument('--summary-only', action='store_true', + help='件数だけ出す (CI 用。ログが公開されるため)') + ap.add_argument('--fail-on-high', action='store_true', + help='risk=HIGH が1件でもあれば終了コード1') + args = ap.parse_args() + + views = collect_views() + imports = collect_imports() + + results = [] + for mod, name, importer, line in imports: + key = (mod, name) + if key not in views: + continue + v = views[key] + if importer == v['file']: + continue # 自分自身の再 import + pos = positional_calls(importer, {name}) + entry = dict(v) + entry['importer'] = importer + entry['import_line'] = line + entry['positional_call_lines'] = pos.get(name, []) + # デコレータ付きのビューを in-process で呼ぶと、そのデコレータが + # 呼び出し元のリクエストコンテキストで動く。位置引数だと + # kwargs しか見ないデコレータが id を取れない (issue62807)。 + if entry['auth_decorators'] and entry['positional_call_lines']: + entry['risk'] = 'HIGH' + elif entry['auth_decorators']: + entry['risk'] = 'MEDIUM' + else: + entry['risk'] = 'LOW' + results.append(entry) + + results.sort(key=lambda e: ({'HIGH': 0, 'MEDIUM': 1, 'LOW': 2}[e['risk']], + e['module'], e['func'])) + + if args.json: + with open(args.json, 'w', encoding='utf-8') as fh: + json.dump(results, fh, ensure_ascii=False, indent=2) + + counts = {r: sum(1 for e in results if e['risk'] == r) + for r in ('HIGH', 'MEDIUM', 'LOW')} + if args.summary_only: + print('in-process から呼ばれるビュー: {} 件 ' + '(HIGH={HIGH} MEDIUM={MEDIUM} LOW={LOW})' + .format(len(results), **counts)) + else: + for e in results: + print('[{risk}] {module}.{func} ({file}:{line})'.format(**e)) + print(' デコレータ: {}'.format(', '.join(e['decorators']) or '-')) + print(' 呼び出し元: {}:{}'.format(e['importer'], e['import_line'])) + if e['positional_call_lines']: + print(' 位置引数での呼び出し: 行 {}'.format( + ', '.join(str(n) for n in e['positional_call_lines']))) + print('\n合計 {} 件 (HIGH={HIGH} MEDIUM={MEDIUM} LOW={LOW})' + .format(len(results), **counts)) + + if args.fail_on_high and counts['HIGH']: + return 1 + return 0 + + +if __name__ == '__main__': + sys.exit(main())