diff --git a/.github/workflows/process.yml b/.github/workflows/process.yml index 8ef99612..d1655b00 100644 --- a/.github/workflows/process.yml +++ b/.github/workflows/process.yml @@ -742,11 +742,26 @@ jobs: name: "Ревью: работа модели" # Единственная недоверенная стадия: модель читает материал и кладёт # запечатанный artifact. Писать в issue и в репозиторий ей незачем — это - # делает `integrate`, проверив происхождение. `id-token` нужен самой - # claude-code-action для авторизации GitHub App. + # делает `integrate`, проверив происхождение: в репозиторий модель не пишет + # вообще, а в issue — только своим комментарием и отдельным issue по §12. + # + # Права здесь реальны только вместе с `github_token` у шага Review. Без него + # claude-code-action меняет OIDC на собственный App-токен с дефолтом + # `contents/issues/pull_requests: write` (`src/github/token.ts`), и блок ниже + # не ограничивает ничего: токен `ghs_…` от claude[bot] лежит прямо в + # окружении Bash-инструмента модели — это поймало ревью r1 по #556 в + # собственной же сессии. С переданным `secrets.GITHUB_TOKEN` обмена не + # происходит, `id-token` больше не нужен, и этот список становится потолком. + # + # `issues: write` остаётся: процесс требует от ревьюера комментарий с + # вердиктом (§7.2) и отдельный issue на Medium вне скоупа (§12). Снять его + # можно только перенеся и то и другое в `integrate` — это отдельная правка + # конвейера, не эта задача. Что остаётся модели этим правом: комментарий, + # метки, правка тела issue. Чего не остаётся: запись в репозиторий, слияние + # (его решает запечатанный verdict.json в `integrate`), релиз. permissions: contents: read - id-token: write + issues: write needs: [guard, prepare] if: needs.prepare.outputs.proceed == 'true' && needs.prepare.outputs.reuse != 'true' runs-on: ubuntu-latest @@ -870,6 +885,11 @@ jobs: # Подписка, а не отдельный счёт API: токен выпускается через # `claude setup-token` (Pro/Max). Действуют лимиты подписки. claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + # Ambient job-scoped токен вместо App-обмена (#556). Со ним + # `permissions:` этой job — настоящий потолок прав модели: ни записи в + # репозиторий, ни постановки метки, ни комментария. Строку нельзя + # снять, не вернув модели право двигать процесс. + github_token: ${{ secrets.GITHUB_TOKEN }} path_to_claude_code_executable: ${{ steps.claude_bin.outputs.path }} prompt: | Ты ревьюер проекта House Plan. Язык ответа — русский. diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 38abad39..d61b6941 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -370,6 +370,29 @@ Two of these are branches upstream, not releases — `home-assistant/actions` branch head was read. They have no tags to follow; the only honest record is "this commit, read on this day". +## What the review model is allowed to do (#556) + +The `model_review` job is the only untrusted stage of the review pipeline: it +runs a model with `Read/Write/Bash` over the material. Its `permissions:` block +is the real ceiling **only because** the `Review` step is handed +`github_token: ${{ secrets.GITHUB_TOKEN }}`. + +Without that input `claude-code-action` exchanges the job's OIDC token for its +own GitHub App installation token (`src/github/token.ts`), and that exchange +defaults to `contents: write`, `pull_requests: write`, `issues: write` no matter +what the calling job declared — the `ghs_…` token then sits in the environment of +the model's own Bash tool. Removing the one line therefore widens the model's +rights silently, with every test still green, which is why there is a witness +(`test/review-doc-guard.test.mjs`) and a mutant +(`review-job-trusts-the-app-token`) standing on it. + +`issues: write` is the single write scope the model keeps, because the process +asks the reviewer for the verdict comment (§7.2) and a separate issue for +out-of-scope Medium findings (§12). It buys comments, labels and issue-body +edits — not a commit, not a merge (that is decided in `integrate` from the sealed +`verdict.json`), not a release. Dropping it means moving both duties into +`integrate`, which is a pipeline change and not part of this one. + ## Dependency and cache gotchas - **polygon-clipping is a trap**: its `.d.ts` declares named exports but the ESM build has only diff --git a/scripts/mutation-gate.mjs b/scripts/mutation-gate.mjs index f77dd822..fddc9860 100644 --- a/scripts/mutation-gate.mjs +++ b/scripts/mutation-gate.mjs @@ -8046,6 +8046,18 @@ const MUTANT_DEFINITIONS = [ replace: "export const isPinned = (spec) => /^[\\w.-]+\\/[\\w./-]+@\\S+$/.test(spec);", }], }, + { + id: 'review-job-trusts-the-app-token', + guard: 'node --test --test-name-pattern="job-scoped" test/review-doc-guard.test.mjs', + because: '#556 r1 H1: без переданного ambient-токена claude-code-action меняет OIDC на ' + + 'собственный App-токен с дефолтом contents/issues/pull_requests: write, и объявленные ' + + '`permissions:` недоверенной стадии перестают быть потолком — молча, зелёным прогоном', + patches: [{ + file: '.github/workflows/process.yml', + find: ' github_token: ${{ secrets.GITHUB_TOKEN }}\n', + replace: '', + }], + }, { id: 'review-integration-trusts-failed-model', guard: 'node --test --test-name-pattern="#551" test/review-doc-guard.test.mjs', diff --git a/test/review-doc-guard.test.mjs b/test/review-doc-guard.test.mjs index 194f65b6..3ff38506 100644 --- a/test/review-doc-guard.test.mjs +++ b/test/review-doc-guard.test.mjs @@ -898,10 +898,10 @@ test('конвейер: права выдаются по job, модель не const jobBlock = (name, next) => workflow.slice(workflow.indexOf(`\n ${name}:\n`), workflow.indexOf(`\n ${next}:\n`)); const model = jobBlock('model_review', 'integrate'); - assert.match(model, /^\s+permissions:\n\s+contents: read\n\s+id-token: write$/m, - 'модели — только чтение и OIDC для claude-code-action'); - assert.doesNotMatch(model.slice(0, model.indexOf('steps:')), /issues: write/, - 'модель не получает права записи в issue'); + assert.match(model, /^\s+permissions:\n\s+contents: read\n\s+issues: write$/m, + 'модели — чтение репозитория и ровно одно право записи: комментарий в issue'); + assert.doesNotMatch(model.slice(0, model.indexOf('steps:')), /contents: write/, + 'модель не получает права записи в репозиторий'); for (const [name, next] of [['guard', 'prepare'], ['prepare', 'model_review']]) { assert.match(jobBlock(name, next), /^\s+permissions:\n\s+contents: read\n\s+issues: write$/m, @@ -910,3 +910,24 @@ test('конвейер: права выдаются по job, модель не const integrate = workflow.slice(workflow.indexOf('\n integrate:\n')); assert.match(integrate, /^\s+permissions:\n\s+contents: read\n\s+issues: write$/m); }); + +// #556 r1 H1: объявленные `permissions:` у model_review ничего не ограничивали, +// пока claude-code-action меняла OIDC на собственный App-токен: его дефолт — +// `contents/issues/pull_requests: write`, и `ghs_…` от claude[bot] лежал прямо в +// окружении Bash-инструмента модели. Потолком права становятся только при +// переданном ambient-токене: `OVERRIDE_GITHUB_TOKEN` замыкает обмен в +// `setupGitHubToken`. Свидетель стоит на проводке, потому что снятие одной +// строки возвращает модели запись в репозиторий молча — прогон остаётся зелёным. +test('ревью: модель работает job-scoped токеном, а не App-обменом (#556)', () => { + const workflow = readFileSync(new URL('../.github/workflows/process.yml', import.meta.url), 'utf8'); + const model = workflow.slice(workflow.indexOf('\n model_review:\n'), workflow.indexOf('\n integrate:\n')); + const review = model.slice(model.indexOf(' - name: Review\n')); + const withBlock = review.slice(review.indexOf(' with:'), review.indexOf(' prompt: |')); + assert.match(withBlock, /^\s+github_token: \$\{\{ secrets\.GITHUB_TOKEN \}\}$/m, + 'шагу Review передан ambient job-scoped токен'); + assert.doesNotMatch(withBlock, /additional_permissions/, + 'права не расширяются через additional_permissions'); + // Без обмена OIDC не нужен, и заявка на него — признак вернувшегося App-токена. + assert.doesNotMatch(model.slice(0, model.indexOf('steps:')), /id-token: write/, + 'OIDC этой стадии больше не выдаётся'); +});