From 88720a80dff6273fe5ae6347b7ddd7047ea9f2e5 Mon Sep 17 00:00:00 2001 From: Evan Hu Date: Sun, 6 Sep 2026 18:15:51 +0900 Subject: [PATCH] =?UTF-8?q?fix(ci):=20=E4=BF=AE=E5=A4=8D=E5=BC=95=E6=93=8E?= =?UTF-8?q?=20bump=20=E8=87=AA=E5=8A=A8=E5=90=88=E5=B9=B6=E7=9A=84?= =?UTF-8?q?=E5=AE=88=E5=8D=AB=EF=BC=8C=E5=B9=B6=E8=A1=A5=E5=9B=9E=20Issue?= =?UTF-8?q?=20=E6=A8=A1=E6=9D=BF=E7=9A=84=E8=84=B1=E6=95=8F=E8=AD=A6?= =?UTF-8?q?=E5=91=8A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit engine-bump-triage.yml 里 `&& steps.lock.outputs.locked == 'yes'` 被写进了 HEAD 这个环境变量的值，而不是步骤的 if:。它在那个位置根本不是条件：不论契约锁校验通过与否，自动合并都会执行；步骤随后发出的评论还会把 commit 后面跟上字面量 `&& steps.lock.outputs.locked == 'yes'` 一起打印出来。因为没有任何东西失败，这个问题没有暴露过。 （实际风险有限：Product inputs 是必需检查，未审的 bump 不会真的被放行。但这条守卫本身写了等于没写。） 补 tests/test_engine_bump_triage.py 三个断言：自动合并的条件必须同时包含判定和锁；任何步骤的 env 值里都不许再出现条件片段；评论用的 BASE/HEAD 必须只是 range 输出本身。第二条是通用的，同类错误以后写在别的步骤上也会被抓到。 同时补回本仓 bug_report.yml 相对组织模板丢掉的两项：日志字段的「不要粘贴真实输入内容、用户词库、API Key 或任何凭据」加粗警告，以及「这不是安全漏洞」的必选确认项。全组织的 Issue 流量几乎都在这个仓库，恰恰是最需要这两条的地方——输入法的日志天然贴近用户真实输入。 --- .github/ISSUE_TEMPLATE/bug_report.yml | 6 ++- .github/workflows/ci.yml | 4 ++ .github/workflows/engine-bump-triage.yml | 8 +++- tests/test_engine_bump_triage.py | 48 ++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 tests/test_engine_bump_triage.py diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index bfa08167..9ad36b4b 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -102,8 +102,8 @@ body: attributes: label: 日志（可选） description: | - 可粘贴相关日志。 - Paste relevant logs, or attach screenshots / recordings. + 可粘贴相关日志或附上截图。**不要粘贴真实输入内容、用户词库、API Key 或任何凭据。** + Paste relevant logs. Do not include real typed text, user dictionary contents, API keys or credentials. render: shell validations: required: false @@ -123,3 +123,5 @@ body: required: true - label: 我已尽量提供可复现信息。I provided enough detail to reproduce the issue. required: true + - label: 这不是安全漏洞。安全问题请按 SECURITY.md 私下上报。This is not a security vulnerability; those go through SECURITY.md. + required: true diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f1caa156..346617e0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,6 +37,10 @@ jobs: run: python3 -m unittest discover -s tests -p 'test_prepare_env.py' - name: Test the automatic release CI gate run: python3 -m unittest discover -s tests -p 'test_release_pr_ci.py' + - name: Test the engine-bump auto-merge guards + run: | + python3 -m pip install --quiet pyyaml + python3 -m unittest discover -s tests -p 'test_engine_bump_triage.py' - name: Read the reviewed lock id: lock run: | diff --git a/.github/workflows/engine-bump-triage.yml b/.github/workflows/engine-bump-triage.yml index b5f93cdc..49000a41 100644 --- a/.github/workflows/engine-bump-triage.yml +++ b/.github/workflows/engine-bump-triage.yml @@ -130,10 +130,14 @@ jobs: fi - name: Enable auto-merge - if: steps.classify.outputs.verdict == 'inert' + # The lock guard belongs here. It used to be appended to the HEAD environment value instead, + # where it was not a condition at all: auto-merge ran whether or not the contract lock + # verified, and the comment below printed the commit followed by the literal text + # "&& steps.lock.outputs.locked == 'yes'". + if: steps.classify.outputs.verdict == 'inert' && steps.lock.outputs.locked == 'yes' env: BASE: ${{ steps.range.outputs.base }} - HEAD: ${{ steps.range.outputs.head }} && steps.lock.outputs.locked == 'yes' + HEAD: ${{ steps.range.outputs.head }} run: | set -euo pipefail gh pr merge --auto --squash "$PR_URL" diff --git a/tests/test_engine_bump_triage.py b/tests/test_engine_bump_triage.py new file mode 100644 index 00000000..38a70862 --- /dev/null +++ b/tests/test_engine_bump_triage.py @@ -0,0 +1,48 @@ +"""Guard the conditions on the engine-bump auto-merge step. + +This exists because of a specific failure: the contract-lock guard was written into the step's HEAD +environment value rather than its `if:`, so it was not a condition at all. Auto-merge ran whether or +not the lock verified, and the comment the step posts printed the commit followed by the literal +text `&& steps.lock.outputs.locked == 'yes'`. Nothing failed, so nothing surfaced it. +""" +from pathlib import Path +import unittest + +import yaml + +WORKFLOW = Path(__file__).resolve().parents[1] / '.github/workflows/engine-bump-triage.yml' + + +def steps(): + # `on:` parses as the boolean True in YAML 1.1, which is why the job lookup goes through 'jobs'. + workflow = yaml.safe_load(WORKFLOW.read_text(encoding='utf-8')) + return [step for job in workflow['jobs'].values() for step in job.get('steps', [])] + + +def step_named(name): + matches = [step for step in steps() if step.get('name') == name] + if len(matches) != 1: + raise AssertionError(f'expected exactly one step named {name!r}, found {len(matches)}') + return matches[0] + + +class AutoMergeGuards(unittest.TestCase): + def test_auto_merge_requires_both_the_verdict_and_the_lock(self): + condition = step_named('Enable auto-merge')['if'] + self.assertIn("steps.classify.outputs.verdict == 'inert'", condition) + self.assertIn("steps.lock.outputs.locked == 'yes'", condition) + + def test_no_step_condition_leaked_into_an_environment_value(self): + for step in steps(): + for key, value in (step.get('env') or {}).items(): + self.assertNotIn('steps.', str(value).split('}}')[-1], + f'{step.get("name")}: env {key} carries a condition fragment') + + def test_the_comment_prints_only_the_commit_range(self): + env = step_named('Enable auto-merge')['env'] + for key in ('BASE', 'HEAD'): + self.assertRegex(env[key], r'^\$\{\{ steps\.range\.outputs\.\w+ \}\}$') + + +if __name__ == '__main__': + unittest.main()