From 10e41ad89fbf008b2960ad179292d2c158ec77a2 Mon Sep 17 00:00:00 2001 From: nevstop Date: Mon, 6 Jul 2026 15:32:52 +0800 Subject: [PATCH 1/3] fix: prevent shell injection by reading user content from env vars Move user-supplied content (comment-body, discussion-title, category-name, comment-author, event-type) from CLI arguments to environment variable fallback in parse_args. This prevents shell injection via backticks or in user content, since these values are no longer expanded by bash when constructing the CLI command. Workflow changes in subsequent commit will remove the corresponding CLI args. --- scripts/router.py | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/scripts/router.py b/scripts/router.py index b5ad1c9..816ab93 100644 --- a/scripts/router.py +++ b/scripts/router.py @@ -1265,26 +1265,26 @@ def parse_args(argv: Optional[list[str]] = None) -> argparse.Namespace: parser.add_argument( "--comment-body", type=str, - default="", - help="评论正文(Webhook 传入,最大 800 字符)", + default=os.environ.get("ROUTER_COMMENT_BODY", ""), + help="评论正文(Webhook 传入,最大 800 字符;缺省时从 ROUTER_COMMENT_BODY 环境变量读取)", ) parser.add_argument( "--discussion-title", type=str, - default="", - help="Discussion 标题(discussion 事件时用于拼接分类输入)", + default=os.environ.get("ROUTER_DISCUSSION_TITLE", ""), + help="Discussion 标题(discussion 事件时用于拼接分类输入;缺省时从 ROUTER_DISCUSSION_TITLE 环境变量读取)", ) parser.add_argument( "--comment-author", type=str, - default="", - help="评论作者用户名", + default=os.environ.get("ROUTER_COMMENT_AUTHOR", ""), + help="评论作者用户名(缺省时从 ROUTER_COMMENT_AUTHOR 环境变量读取)", ) parser.add_argument( "--category-name", type=str, - default="", - help="Discussion 所属分类名", + default=os.environ.get("ROUTER_CATEGORY_NAME", ""), + help="Discussion 所属分类名(缺省时从 ROUTER_CATEGORY_NAME 环境变量读取)", ) parser.add_argument( "--dry-run", @@ -1295,8 +1295,8 @@ def parse_args(argv: Optional[list[str]] = None) -> argparse.Namespace: parser.add_argument( "--event-type", type=str, - default="", - help="GitHub 事件类型(discussion / discussion_comment)", + default=os.environ.get("ROUTER_EVENT_TYPE", ""), + help="GitHub 事件类型(discussion / discussion_comment;缺省时从 ROUTER_EVENT_TYPE 环境变量读取)", ) parser.add_argument( "--classify-only", @@ -1395,7 +1395,11 @@ def main(argv: Optional[list[str]] = None) -> int: # 3. --classify-only:仅输出意图供 workflow 捕获(token 仅用于获取讨论上下文,非必需) if args.classify_only: - print(intent) + try: + print(intent) + except Exception: + logger.exception("classify-only 模式输出异常") + print("OTHER") return 0 # ── 后续操作需要 token ──────────────────────────────────────────── @@ -1428,4 +1432,9 @@ def main(argv: Optional[list[str]] = None) -> int: if __name__ == "__main__": - sys.exit(main()) + try: + sys.exit(main()) + except Exception: + logger.exception("Router 执行异常,输出 fallback 结果") + print("OTHER") + sys.exit(0) From f6fff3fc1435e3e0e707f0ac1166f8a1ac6a7407 Mon Sep 17 00:00:00 2001 From: nevstop Date: Mon, 6 Jul 2026 15:32:55 +0800 Subject: [PATCH 2/3] fix: remove user-content CLI args from workflow, add actions:write, upgrade actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Remove all user-supplied content CLI args (--comment-body, --discussion-title, --category-name, --event-type, --comment-author) from the 4 router.py invocations — these are now read from environment variables instead, eliminating shell injection risk. 2. Add 'actions: write' permission to allow cache save operations to succeed. 3. Upgrade actions/setup-python@v5 → v6, actions/cache/{restore,save}@v4 → v5 to resolve Node.js 20 deprecation warnings. --- .github/workflows/org-router.yml | 32 +++++++++----------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/.github/workflows/org-router.yml b/.github/workflows/org-router.yml index 26a8e4e..4dcfcc3 100644 --- a/.github/workflows/org-router.yml +++ b/.github/workflows/org-router.yml @@ -55,6 +55,7 @@ jobs: permissions: contents: read discussions: write + actions: write steps: # ── 基础环境 ────────────────────────────────────────────────────── @@ -62,13 +63,13 @@ jobs: with: token: ${{ secrets.CSM_QA_GH_TOKEN }} - - uses: actions/setup-python@v5 + - uses: actions/setup-python@v6 with: python-version: '3.11' - name: Restore venv cache id: cache-venv - uses: actions/cache/restore@v4 + uses: actions/cache/restore@v5 with: path: .venv key: ${{ runner.os }}-venv-py3.11-${{ hashFiles('requirements-bot.txt') }} @@ -90,7 +91,7 @@ jobs: - name: Save venv cache if: always() && steps.install-deps.outcome == 'success' && steps.cache-venv.outputs.cache-hit != 'true' - uses: actions/cache/save@v4 + uses: actions/cache/save@v5 with: path: .venv key: ${{ steps.cache-venv.outputs.cache-primary-key }} @@ -115,12 +116,7 @@ jobs: run: | INTENT=$(python scripts/router.py \ --classify-only \ - --discussion-number "${ROUTER_DISCUSSION_NUMBER}" \ - --comment-body "${ROUTER_COMMENT_BODY}" \ - --discussion-title "${ROUTER_DISCUSSION_TITLE}" \ - --category-name "${ROUTER_CATEGORY_NAME}" \ - --event-type "${ROUTER_EVENT_TYPE}" \ - --comment-author "${ROUTER_COMMENT_AUTHOR}") + --discussion-number "${ROUTER_DISCUSSION_NUMBER}") echo "result=${INTENT}" >> "$GITHUB_OUTPUT" echo "Intent: ${INTENT}" @@ -147,9 +143,6 @@ jobs: python scripts/router.py \ --intent JOIN \ --discussion-number "${ROUTER_DISCUSSION_NUMBER}" \ - --comment-body "${ROUTER_COMMENT_BODY}" \ - --comment-author "${ROUTER_COMMENT_AUTHOR}" \ - --event-type "${ROUTER_EVENT_TYPE}" \ ${{ github.event.inputs.dry_run == 'true' && '--dry-run' || '' }} # ════════════════════════════════════════════════════════════════════ @@ -161,7 +154,7 @@ jobs: - name: Restore HuggingFace models cache id: cache-hf if: steps.classify.outputs.result == 'QA' - uses: actions/cache/restore@v4 + uses: actions/cache/restore@v5 with: path: ~/.cache/huggingface key: ${{ runner.os }}-hf-BAAI-bge-small-zh-v1.5 @@ -194,7 +187,7 @@ jobs: - name: Restore vector store cache id: cache-vs if: steps.classify.outputs.result == 'QA' - uses: actions/cache/restore@v4 + uses: actions/cache/restore@v5 with: path: | .csm_llm_qa/vector_store @@ -221,10 +214,6 @@ jobs: python scripts/router.py \ --intent QA \ --discussion-number "${ROUTER_DISCUSSION_NUMBER}" \ - --comment-body "${ROUTER_COMMENT_BODY}" \ - --category-name "${ROUTER_CATEGORY_NAME}" \ - --event-type "${ROUTER_EVENT_TYPE}" \ - --comment-author "${ROUTER_COMMENT_AUTHOR}" \ ${{ github.event.inputs.dry_run == 'true' && '--dry-run' || '' }} # QA 之后才保存 RAG 缓存 @@ -242,7 +231,7 @@ jobs: - name: Save HuggingFace models cache if: always() && steps.classify.outputs.result == 'QA' && steps.cache-hf.outputs.cache-hit != 'true' && steps.check-hf-populated.outputs.populated == 'true' - uses: actions/cache/save@v4 + uses: actions/cache/save@v5 with: path: ~/.cache/huggingface key: ${{ steps.cache-hf.outputs.cache-primary-key }} @@ -274,7 +263,7 @@ jobs: - name: Save vector store cache if: always() && steps.classify.outputs.result == 'QA' && steps.cache-vs.outputs.cache-hit != 'true' && steps.check-vs-populated.outputs.populated == 'true' - uses: actions/cache/save@v4 + uses: actions/cache/save@v5 with: path: | .csm_llm_qa/vector_store @@ -300,7 +289,4 @@ jobs: python scripts/router.py \ --intent OTHER \ --discussion-number "${ROUTER_DISCUSSION_NUMBER}" \ - --comment-body "${ROUTER_COMMENT_BODY}" \ - --event-type "${ROUTER_EVENT_TYPE}" \ - --comment-author "${ROUTER_COMMENT_AUTHOR}" \ ${{ github.event.inputs.dry_run == 'true' && '--dry-run' || '' }} From 79234bf107160ad6d2c1cc7dd607fe8c3df4bef1 Mon Sep 17 00:00:00 2001 From: nevstop Date: Mon, 6 Jul 2026 15:36:57 +0800 Subject: [PATCH 3/3] fix: set caplog level to INFO in human-intervention tests The 3 tests in TestMainHumanIntervention check for INFO-level log messages via caplog, but caplog's default handler level is WARNING. Add caplog.set_level(logging.INFO) so these messages are captured. --- tests/test_router.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_router.py b/tests/test_router.py index 88dcbca..0296560 100644 --- a/tests/test_router.py +++ b/tests/test_router.py @@ -2,6 +2,8 @@ from __future__ import annotations +import logging + import pytest from unittest.mock import patch, MagicMock @@ -948,6 +950,7 @@ def _argv_other(**overrides) -> list[str]: def test_other_skipped_when_human_intervention(self, monkeypatch, caplog): """OTHER 路径下,thread 已有人工介入 → 跳过回复。""" + caplog.set_level(logging.INFO, logger="org_router") monkeypatch.setattr("scripts.router.LLM_API_KEY", "") monkeypatch.setenv("CSM_QA_GH_TOKEN", "fake-token") monkeypatch.setattr( @@ -976,6 +979,7 @@ def mock_post(*args, **kwargs): def test_qa_guide_skipped_when_human_intervention(self, monkeypatch, caplog): """_handle_qa 非 Q&A 引导路径下,thread 已有人工介入 → 跳过回复。""" + caplog.set_level(logging.INFO, logger="org_router") monkeypatch.setattr("scripts.router.LLM_API_KEY", "") monkeypatch.setenv("CSM_QA_GH_TOKEN", "fake-token") monkeypatch.setattr( @@ -1041,6 +1045,7 @@ def mock_post(*args, **kwargs): def test_join_skipped_when_human_intervention(self, monkeypatch, caplog): """JOIN 路径下,thread 已有人工介入 → 跳过处理。""" + caplog.set_level(logging.INFO, logger="org_router") monkeypatch.setattr("scripts.router.LLM_API_KEY", "") monkeypatch.setenv("CSM_QA_GH_TOKEN", "fake-token") monkeypatch.setattr(