diff --git a/.github/workflows/close-empty-pr.yml b/.github/workflows/close-empty-pr.yml deleted file mode 100644 index 530384a2..00000000 --- a/.github/workflows/close-empty-pr.yml +++ /dev/null @@ -1,55 +0,0 @@ -# Auto-closes pull requests that have commits but no net change vs. their base -# (GitHub shows "No files changed / +0 -0"). The org's bot authors sometimes -# open such empty PRs; this closes them so humans do not have to. -# -# Runs per repo as a central required org workflow. pull_request_target gives a -# write-scoped token (needed to close) without checking out untrusted PR code, -# so there is no code-execution risk — the job only reads PR metadata and closes. -# Drafts are left alone. -name: Close Empty PR - -on: - pull_request_target: - types: [opened, synchronize, reopened, ready_for_review] - -permissions: - pull-requests: write - contents: read - -jobs: - close-empty: - runs-on: ubuntu-latest - steps: - - name: Close PR when it has no net changes - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.event.pull_request.base.repo.full_name }} - PR: ${{ github.event.pull_request.number }} - run: | - set -euo pipefail - - # GitHub computes the diff asynchronously; poll briefly for a settled - # changed_files count before deciding (null while still computing). - changed="" - draft="false" - for _ in 1 2 3 4 5 6; do - payload="$(gh api "repos/${REPO}/pulls/${PR}")" - changed="$(jq -r '.changed_files // ""' <<<"$payload")" - draft="$(jq -r '.draft // false' <<<"$payload")" - [ -n "$changed" ] && break - sleep 10 - done - echo "PR #${PR} changed_files=${changed:-unknown} draft=${draft}" - - if [ "$draft" = "true" ]; then - echo "Draft PR — leaving it open." - exit 0 - fi - if [ "$changed" = "0" ]; then - gh pr comment "${PR}" --repo "${REPO}" \ - --body "자동 정리: base 대비 실제 변경(diff)이 0건이라 이 PR을 닫습니다. 변경을 추가한 뒤 reopen하세요." || true - gh pr close "${PR}" --repo "${REPO}" - echo "Closed empty PR #${PR}." - else - echo "PR has ${changed} changed file(s); leaving it open." - fi diff --git a/.github/workflows/codeql-pr.yml b/.github/workflows/codeql-pr.yml deleted file mode 100644 index 85145296..00000000 --- a/.github/workflows/codeql-pr.yml +++ /dev/null @@ -1,124 +0,0 @@ -# Uploads CodeQL code scanning analyses on every PR so the org ruleset -# "CWL Central required workflows" -> code_scanning(CodeQL) can evaluate -# mergeability. Without PR-head and merge-preview SARIF on merge_commit_sha, -# approved PRs stay mergeStateStatus=BLOCKED with -# "Code scanning is waiting for results from CodeQL". -name: CodeQL PR - -on: - pull_request: - branches: [main, master, develop] - -permissions: - contents: read - -jobs: - detect-languages: - name: Detect CodeQL languages - runs-on: ubuntu-latest - outputs: - matrix: ${{ steps.detect.outputs.matrix }} - steps: - - name: Checkout PR head - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - with: - persist-credentials: false - ref: ${{ github.event.pull_request.head.sha }} - - - name: Build language matrix - id: detect - run: | - matrix='[]' - if [ -d .github/workflows ]; then - matrix=$(echo "$matrix" | jq -c '. + [{"language":"actions","build-mode":"none"}]') - fi - if find . -type f \( -name '*.js' -o -name '*.jsx' -o -name '*.ts' -o -name '*.tsx' \) \ - -not -path './.git/*' | head -1 | grep -q .; then - matrix=$(echo "$matrix" | jq -c '. + [{"language":"javascript-typescript","build-mode":"none"}]') - fi - if find . -type f -name '*.py' -not -path './.git/*' | head -1 | grep -q .; then - matrix=$(echo "$matrix" | jq -c '. + [{"language":"python","build-mode":"none"}]') - fi - if [ "$(echo "$matrix" | jq 'length')" -eq 0 ]; then - matrix='[{"language":"actions","build-mode":"none"}]' - fi - { - echo 'matrix<> "$GITHUB_OUTPUT" - - analyze-head: - name: CodeQL compatibility analysis (${{ matrix.language }}) - needs: detect-languages - runs-on: ubuntu-latest - permissions: - actions: read - contents: read - security-events: write - strategy: - fail-fast: false - matrix: ${{ fromJSON(needs.detect-languages.outputs.matrix) }} - steps: - - name: Harden the runner (Audit all outbound calls) - uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 - with: - egress-policy: audit - - - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - with: - persist-credentials: false - ref: ${{ github.event.pull_request.head.sha }} - - - name: Initialize CodeQL - uses: github/codeql-action/init@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4.36.2 - with: - languages: ${{ matrix.language }} - build-mode: ${{ matrix.build-mode }} - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4.36.2 - with: - category: "/language:${{ matrix.language }}" - upload: always - ref: ${{ format('refs/pull/{0}/head', github.event.pull_request.number) }} - sha: ${{ github.event.pull_request.head.sha }} - - analyze-merge: - name: CodeQL merge preview (${{ matrix.language }}) - needs: detect-languages - if: github.event.pull_request.merge_commit_sha != '' - runs-on: ubuntu-latest - permissions: - actions: read - contents: read - security-events: write - strategy: - fail-fast: false - matrix: ${{ fromJSON(needs.detect-languages.outputs.matrix) }} - steps: - - name: Harden the runner (Audit all outbound calls) - uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 - with: - egress-policy: audit - - - name: Checkout merge preview - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - with: - persist-credentials: false - ref: ${{ format('refs/pull/{0}/merge', github.event.pull_request.number) }} - - - name: Initialize CodeQL - uses: github/codeql-action/init@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4.36.2 - with: - languages: ${{ matrix.language }} - build-mode: ${{ matrix.build-mode }} - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4.36.2 - with: - category: "/language:${{ matrix.language }}-merge" - upload: always - ref: ${{ format('refs/pull/{0}/merge', github.event.pull_request.number) }} - sha: ${{ github.event.pull_request.merge_commit_sha }} \ No newline at end of file diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index a32b3809..2ab6028a 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -2007,10 +2007,10 @@ jobs: }, "agent": { "ci-review": { - "description": "Thorough read-only CI pull request reviewer", + "description": "Compact read-only CI pull request reviewer", "mode": "primary", "prompt": "{file:./ci-review-prompt.md}", - "steps": 100, + "steps": 4, "reasoningEffort": "high", "permission": { "edit": "deny", @@ -2030,7 +2030,7 @@ jobs: "description": "Expanded read-only CI pull request reviewer fallback", "mode": "primary", "prompt": "{file:./ci-review-prompt.md}", - "steps": 150, + "steps": 12, "reasoningEffort": "high", "permission": { "edit": "deny", @@ -2050,7 +2050,7 @@ jobs: "description": "Use this subagent immediately after code changes, before opening or merging a PR, or when asked to review a diff. Reviews only; never edits code. Focuses on correctness, security, maintainability, tests, and production risk.", "mode": "subagent", "prompt": "{file:./code-reviewer-prompt.md}", - "steps": 100, + "steps": 16, "color": "#7c3aed", "reasoningEffort": "high", "permission": { @@ -2272,7 +2272,7 @@ jobs: - name: Run OpenCode PR Review model pool id: opencode_review_model_pool if: needs.coverage-evidence.result == 'success' - timeout-minutes: 350 + timeout-minutes: 285 env: STRIX_GITHUB_MODELS_TOKEN: ${{ secrets.STRIX_GITHUB_MODELS_TOKEN || github.token }} GITHUB_TOKEN: ${{ secrets.STRIX_GITHUB_MODELS_TOKEN || github.token }} @@ -2280,9 +2280,9 @@ jobs: SHARE: "false" NPM_CONFIG_IGNORE_SCRIPTS: "true" NO_COLOR: "1" - OPENCODE_MODEL_CANDIDATES: "github-models/openai/gpt-5 github-models/openai/gpt-5-chat github-models/deepseek/deepseek-v3-0324 github-models/openai/o3 github-models/deepseek/deepseek-r1 github-models/openai/o4-mini github-models/openai/o3-mini github-models/openai/gpt-5-mini github-models/mistral-ai/mistral-medium-2505 github-models/openai/gpt-5-nano github-models/deepseek/deepseek-r1-0528 github-models/meta/llama-4-maverick-17b-128e-instruct-fp8 github-models/meta/llama-4-scout-17b-16e-instruct" - OPENCODE_MODEL_ATTEMPTS: "5" - OPENCODE_RUN_TIMEOUT_SECONDS: "20400" + OPENCODE_MODEL_CANDIDATES: "github-models/openai/o4-mini github-models/openai/o3-mini github-models/openai/gpt-5-mini github-models/openai/gpt-5-chat github-models/openai/o3 github-models/mistral-ai/mistral-medium-2505 github-models/openai/gpt-5-nano github-models/deepseek/deepseek-r1-0528 github-models/deepseek/deepseek-r1 github-models/deepseek/deepseek-v3-0324 github-models/meta/llama-4-maverick-17b-128e-instruct-fp8 github-models/meta/llama-4-scout-17b-16e-instruct" + OPENCODE_MODEL_ATTEMPTS: "1" + OPENCODE_RUN_TIMEOUT_SECONDS: "600" OPENCODE_EXPORT_TIMEOUT_SECONDS: "120" OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "0" OPENCODE_BACKOFF_INITIAL_SECONDS: "30" diff --git a/.github/workflows/osv-scanner-pr.yml b/.github/workflows/osv-scanner-pr.yml index b3c8a58b..00d2b3d5 100644 --- a/.github/workflows/osv-scanner-pr.yml +++ b/.github/workflows/osv-scanner-pr.yml @@ -6,7 +6,7 @@ name: OSV-Scanner PR on: pull_request: - branches: [main, master, develop] + branches: [main] permissions: # Upload SARIF to Security > Code Scanning. See github/codeql-action#2117. diff --git a/.github/workflows/scorecard-pr.yml b/.github/workflows/scorecard-pr.yml index f5436bfa..15771dc4 100644 --- a/.github/workflows/scorecard-pr.yml +++ b/.github/workflows/scorecard-pr.yml @@ -13,7 +13,7 @@ name: Scorecard PR on: pull_request: - branches: [main, master, develop] + branches: [main] permissions: contents: read diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml deleted file mode 100644 index 908a07b0..00000000 --- a/.github/workflows/security-scan.yml +++ /dev/null @@ -1,111 +0,0 @@ -# Central bundled security gate for every ContextualWisdomLab repo. -# -# This is a REQUIRED org workflow (see the "CWL Central required workflows" -# ruleset). It bundles the supply-chain / vulnerability / posture scanners into -# one gate so they pass or fail as a unit: -# -# osv-scan HARD diff-scoped — fails on NEW vulns the PR introduces -# dependency-review HARD diff-scoped — fails on vulnerable/denied deps the PR adds -# trivy-fs HARD repo-wide — fails on FIXABLE CRITICAL/HIGH findings -# scorecard SOFT repo posture — uploaded for visibility, never blocks -# -# Gating is by the JOB result (a failed job fails this required workflow -> -# merge blocked), NOT by the code_scanning ruleset rule. The code_scanning rule -# stays CodeQL-only on purpose: requiring multiple code-scanning TOOLS there is -# unsatisfiable because default-setup CodeQL uploads to refs/pull/N/head while -# pull_request workflows upload to refs/pull/N/merge, so no single ref ever holds -# all tools. Bundling at the workflow/check level is ref-independent. -# -# NOTE on trivy-fs: it scans the whole repo, so a pre-existing FIXABLE -# CRITICAL/HIGH finding blocks every PR in that repo until it is fixed. Flip its -# `exit-code` to "0" to make Trivy visibility-only if that is too strict. -name: Security Scan - -on: - pull_request: - branches: [main, master, develop] - -permissions: - actions: read - contents: read - security-events: write - -jobs: - osv-scan: - # ponytail: reuse upstream diff-scoped PR scanner instead of hand-rolling it - uses: google/osv-scanner-action/.github/workflows/osv-scanner-reusable-pr.yml@9a498708959aeaef5ef730655706c5a1df1edbc2 # v2.3.8 - permissions: - actions: read - contents: read - security-events: write - with: - fail-on-vuln: true - - dependency-review: - runs-on: ubuntu-latest - permissions: - contents: read - steps: - - name: Checkout - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - with: - persist-credentials: false - - name: Dependency review - uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 - with: - fail-on-severity: high - comment-summary-in-pr: on-failure - - trivy-fs: - runs-on: ubuntu-latest - permissions: - contents: read - security-events: write - actions: read - steps: - - name: Checkout - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - with: - persist-credentials: false - - name: Trivy filesystem scan - uses: aquasecurity/trivy-action@a9c7b0f06e461e9d4b4d1711f154ee024b8d7ab8 # v0.36.0 - with: - scan-type: fs - scan-ref: . - scanners: vuln,secret,misconfig - severity: CRITICAL,HIGH - ignore-unfixed: true - format: sarif - output: trivy-results.sarif - exit-code: "1" - - name: Upload Trivy SARIF to code scanning - if: always() && hashFiles('trivy-results.sarif') != '' - uses: github/codeql-action/upload-sarif@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4.36.2 - with: - sarif_file: trivy-results.sarif - category: trivy-fs - - scorecard: - runs-on: ubuntu-latest - # SOFT: posture findings are unrelated to the PR diff, so never block merge. - continue-on-error: true - permissions: - security-events: write - contents: read - actions: read - steps: - - name: Checkout - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - with: - persist-credentials: false - - name: Run Scorecard - uses: ossf/scorecard-action@4eaacf0543bb3f2c246792bd56e8cdeffafb205a # v2.4.3 - with: - results_file: results.sarif - results_format: sarif - publish_results: false - - name: Upload Scorecard SARIF to code scanning - uses: github/codeql-action/upload-sarif@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4.36.2 - with: - sarif_file: results.sarif - category: scorecard diff --git a/.jules/bolt.md b/.jules/bolt.md index 4bc70515..a035da6f 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -25,6 +25,3 @@ ## 2024-05-19 - Pre-compile Regex Patterns in Loop-called Functions **Learning:** In `scripts/ci/pr_review_merge_scheduler.py`, the `scrub_sensitive_data` function was repeatedly compiling multiple regex patterns via `re.sub` for every log line or text scrubbed. This incurs measurable overhead due to cache lookups and object recreation in tightly looped string processing. **Action:** When using multiple regex replacements inside functions that are called frequently or process large amounts of text, define and pre-compile the regex objects at the module level (e.g., `SENSITIVE_DATA_SCRUB_PATTERNS`) and iterate over them using `pattern.sub()`. -## 2026-07-02 - Credential Masking Security Hole in Subprocess Environments -**Learning:** Found a critical missing credential masking pattern in `scripts/ci/noema_review_gate.py`'s `scrub_sensitive_data` which didn't mask `Authorization: Basic` or `Proxy-Authorization: Basic` tokens unlike its analogous helper in `scripts/ci/pr_review_merge_scheduler.py`. This leaves exception messages and logs vulnerable to exposing sensitive credentials when HTTP operations fail. -**Action:** When implementing credential masking functions that sanitize tracebacks and log messages, ensure the masking scope includes all relevant headers, particularly `Authorization` and `Proxy-Authorization`. Ensure parity across masking helpers across CI scripts to prevent blind spots. diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 7039d665..9133bba1 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -22,7 +22,3 @@ **Vulnerability:** Server-Side Request Forgery (SSRF) / Local File Inclusion **Learning:** Functions that fetch URLs provided via user inputs (e.g., `wait_for_url` fetching `--backend-ready-url` in CI scripts) can inadvertently read local files if they do not validate the scheme. Python's `urllib.request.urlopen` supports `file://` schemes, allowing attackers to access arbitrary file contents from the host machine or sandbox if they can control the URL parameter. **Prevention:** Always validate URL inputs to restrict allowed schemes. Check that URLs explicitly start with `http://` or `https://` before fetching them with standard libraries like `urllib`. -## 2026-07-03 - Prevent SSRF via URL Scheme Validation -**Vulnerability:** Server-Side Request Forgery (SSRF) / Local File Inclusion -**Learning:** External URL fetching with `urllib.request.urlopen` (like API endpoints passed via environment variables) can accept schemes like `file://` implicitly, which could allow arbitrary file reading or internal network scanning if the environment is misconfigured or manipulated. -**Prevention:** Always validate that URLs explicitly start with `http://` or `https://` before using them in standard library requests. Append to suppress linter warnings only after verifying the input is validated. diff --git a/ci-review-prompt.md b/ci-review-prompt.md index f1aa64e6..31bab2bd 100644 --- a/ci-review-prompt.md +++ b/ci-review-prompt.md @@ -137,34 +137,6 @@ names such as `id`, `name`, `type`, `value`, `data`, `user`, `order`, `group`, or `key` when a two-word snake_case, camelCase, PascalCase, or local-equivalent name would reduce ORM, SQL reserved-word, serialization, or portability risk. -Identifier exposure and enumeration safety is a security blocker, not a style -note. When a primary key or any identifier that appears in an API response, URL -path or query, redirect, filename, cache key, or other client-visible surface -is a sequential or auto-incrementing integer (SERIAL/BIGSERIAL, AUTO_INCREMENT, -IDENTITY, or an ORM auto-increment `id`), return REQUEST_CHANGES: sequential -ids let attackers enumerate and reach other records (IDOR/enumeration — the -Coupang breach exploited guessable sequential ids). Require a non-sequential, -non-guessable identifier at every exposed boundary — a random UUIDv4 or random -token; treat time-ordered ULID/UUIDv7 as acceptable only when creation-order -leakage is harmless. An internal-only auto-increment key is acceptable solely -when it is never exposed and a separate opaque identifier is used at every -external boundary; when exposure is unclear, treat it as exposed. - -Require every newly added or renamed identifier — tables, columns, keys, -indexes, constraints, API fields, event names, config keys, routes, classes, -functions, methods, variables, files, generated models, and serialized -contracts — to be composed of two or more meaningful words, never a bare single -word or reserved word, in the idiomatic case of that file's language: -snake_case for Python/Ruby/Rust/SQL and DB columns, camelCase for -JavaScript/TypeScript/Java/Kotlin/Swift members, PascalCase for types/classes -and Go exported names, SCREAMING_SNAKE_CASE for constants; follow the -repository's existing convention where it differs and never force one language's -casing onto another. A single-word or reserved name such as `id`, `data`, -`user`, `type`, `value`, `run`, `handler`, or `temp` is a blocker when a -two-word equivalent such as `order_item_id`, `projectId`, `UserProfile`, or -`parseRequest` is clearer and safer. Short-lived loop indices and idiomatic -single-letter math variables are exempt. - Use these severity meanings in human-readable findings and in the control block: diff --git a/code-reviewer-prompt.md b/code-reviewer-prompt.md index 01c34e50..25dc4814 100644 --- a/code-reviewer-prompt.md +++ b/code-reviewer-prompt.md @@ -147,34 +147,6 @@ names such as `id`, `name`, `type`, `value`, `data`, `user`, `order`, `group`, or `key` when a two-word snake_case, camelCase, PascalCase, or local-equivalent name would reduce ORM, SQL reserved-word, serialization, or portability risk. -Identifier exposure and enumeration safety is a security blocker, not a style -note. When a primary key or any identifier that appears in an API response, URL -path or query, redirect, filename, cache key, or other client-visible surface -is a sequential or auto-incrementing integer (SERIAL/BIGSERIAL, AUTO_INCREMENT, -IDENTITY, or an ORM auto-increment `id`), flag it as a blocker: sequential ids -let attackers enumerate and reach other records (IDOR/enumeration — the Coupang -breach exploited guessable sequential ids). Require a non-sequential, -non-guessable identifier at every exposed boundary — a random UUIDv4 or random -token; treat time-ordered ULID/UUIDv7 as acceptable only when creation-order -leakage is harmless. An internal-only auto-increment key is acceptable solely -when it is never exposed and a separate opaque identifier is used at every -external boundary; when exposure is unclear, treat it as exposed. - -Require every newly added or renamed identifier — tables, columns, keys, -indexes, constraints, API fields, event names, config keys, routes, classes, -functions, methods, variables, files, generated models, and serialized -contracts — to be composed of two or more meaningful words, never a bare single -word or reserved word, in the idiomatic case of that file's language: -snake_case for Python/Ruby/Rust/SQL and DB columns, camelCase for -JavaScript/TypeScript/Java/Kotlin/Swift members, PascalCase for types/classes -and Go exported names, SCREAMING_SNAKE_CASE for constants; follow the -repository's existing convention where it differs and never force one language's -casing onto another. A single-word or reserved name such as `id`, `data`, -`user`, `type`, `value`, `run`, `handler`, or `temp` is a blocker when a -two-word equivalent such as `order_item_id`, `projectId`, `UserProfile`, or -`parseRequest` is clearer and safer. Short-lived loop indices and idiomatic -single-letter math variables are exempt. - Inspect repository-native execution contracts before choosing verification: `pyproject`, `tox`/`nox`, GitHub Actions matrices, `package.json`/engines/ `.nvmrc`, `Cargo.toml`, `go.mod`, Maven/Gradle files, R `DESCRIPTION`, diff --git a/docs/org-required-workflow-rollout.md b/docs/org-required-workflow-rollout.md index 4d910e26..1f1a48df 100644 --- a/docs/org-required-workflow-rollout.md +++ b/docs/org-required-workflow-rollout.md @@ -16,9 +16,6 @@ Use an organization repository ruleset instead of copying workflow files into ea - `.github/workflows/strix.yml` - `.github/workflows/opencode-review.yml` - `.github/workflows/pr-review-merge-scheduler.yml` - - `.github/workflows/osv-scanner-pr.yml` - - `.github/workflows/scorecard-pr.yml` - - `.github/workflows/codeql-pr.yml` - Required workflow ref: `refs/heads/main` - Last verified workflow implementation base commit: `ef9950e6b55bf943c0295e1df3e34c94210d21cc` (`#283`) - Required workflow trigger support: `pull_request_target`, `push`, `workflow_run` @@ -47,44 +44,6 @@ The central `.github/workflows/opencode-review.yml` is now part of the active or Keep the OpenCode required workflow active only while the central workflow keeps proving current-head coverage, CodeGraph initialization, bounded evidence, model review output, and approval-gate publication on the current head. -## Code scanning required workflow posture - -The central `.github/workflows/codeql-pr.yml`, `.github/workflows/scorecard-pr.yml`, -and `.github/workflows/osv-scanner-pr.yml` workflows supply PR-head and merge-preview -code scanning analyses for ruleset `18156473` `code_scanning` (CodeQL, Scorecard, -osv-scanner). They trigger on pull requests to `main`, `master`, and `develop` so -Git Flow repositories on `develop` inherit the same merge gate as GitHub Flow repos. - -CodeQL merge preview checks out `refs/pull//merge` and uploads SARIF with -`sha: pull_request.merge_commit_sha` because the ruleset evaluates that commit, -not the ephemeral merge ref OID. - -Repository-local `codeql.yml` push/default-branch scans may remain for branch -history, but PR merge gates should rely on the central `codeql-pr.yml` workflow. - -### Repository-local CodeQL inventory (2026-07-04) - -Org audit of default-branch workflow files. Repos without any local CodeQL -workflow depend entirely on central `codeql-pr.yml` once ruleset `18156473` -includes that path; they are the most exposed to -`Code scanning is waiting for results from CodeQL` until the ruleset update -lands. - -| Repository | Default branch | Local CodeQL workflow | PR trigger | merge_commit_sha SARIF | -| --- | --- | --- | ---: | ---: | -| `aFIPC` | `master` | `codeql.yml` | yes | no | -| `bandscope` | `develop` | `codeql.yml` | yes | no | -| `newsdom-api` | `develop` | `codeql.yml` | yes | no | -| `pg-erd-cloud` | `main` | `codeql.yml`, `codeql-backfill.yml` | yes (`codeql.yml`) | no | -| `xtrmLLMBatchPython` | `develop` | `codeql.yml` | yes | no | -| `naruon` | `develop` | `codeql.yml` | yes (temporary; PR `#916` retires PR trigger) | yes (repo-local interim fix) | -| all other public non-fork org repos | varies | none observed | — | — | - -No repository-local PR CodeQL workflow besides `naruon` uploads merge-preview -SARIF on `merge_commit_sha`. Centralizing through `codeql-pr.yml` fixes every -inherited repository in one ruleset change; per-repo deletion of PR triggers is -optional cleanup to avoid duplicate scans. - ## Scheduler required workflow posture The central `.github/workflows/pr-review-merge-scheduler.yml` is now part of the active organization required workflow ruleset. diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 621e4506..a022744b 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -4,14 +4,11 @@ from __future__ import annotations import argparse -import ipaddress import json import os import re -import socket import subprocess import sys -import urllib.parse import urllib.request from collections.abc import Sequence from typing import Any @@ -36,25 +33,18 @@ RUNNING_STATES = {"QUEUED", "IN_PROGRESS", "PENDING", "REQUESTED", "WAITING", "EXPECTED"} MAX_DIFF_CHARS = 60000 -# ⚡ Bolt: Pre-compiled regex patterns to avoid recompilation on every scrub_sensitive_data call. -# Impact: Improves string processing performance in error reporting. -SENSITIVE_DATA_SCRUB_PATTERNS = ( - (re.compile(r'(?i)(bearer\s+)[^\s"\'\\]+'), r'\1***'), - (re.compile(r'(?i)(token\s+)[^\s"\'\\]+'), r'\1***'), - (re.compile(r'(?i)\b(?:github_pat_[A-Za-z0-9_]+|gh[pousr]_[A-Za-z0-9_]+)\b'), '***'), - (re.compile(r'\b(sk-[A-Za-z0-9_-]+)'), '***'), - (re.compile(r'\b(xox[baprs]-[A-Za-z0-9-]+)'), '***'), - (re.compile(r'\b(AKIA[0-9A-Z]{16})'), '***'), - (re.compile(r'(?i)((?:api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|client[_-]?secret|password|passwd|secret)\s*[:=]\s*)["\']?[^"\'\s]+["\']?'), r'\1***'), - (re.compile(r'(?i)((?:authorization|proxy-authorization)\s*:\s*(?:bearer|basic)\s+)[A-Za-z0-9._~+\/=-]+'), r'\1***'), -) def scrub_sensitive_data(text: str | None) -> str | None: """Mask sensitive tokens in text to prevent secret leakage.""" if not text: return text - for pattern, repl in SENSITIVE_DATA_SCRUB_PATTERNS: - text = pattern.sub(repl, text) + text = re.sub(r'(?i)(bearer\s+)[^\s"\'\\]+', r'\1***', text) + text = re.sub(r'(?i)(token\s+)[^\s"\'\\]+', r'\1***', text) + text = re.sub(r'(?i)\b(?:github_pat_[A-Za-z0-9_]+|gh[pousr]_[A-Za-z0-9_]+)\b', '***', text) + text = re.sub(r'\b(sk-[A-Za-z0-9_-]+)', '***', text) + text = re.sub(r'\b(xox[baprs]-[A-Za-z0-9-]+)', '***', text) + text = re.sub(r'\b(AKIA[0-9A-Z]{16})', '***', text) + text = re.sub(r'(?i)((?:api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|client[_-]?secret|password|passwd|secret)\s*[:=]\s*)["\']?[^"\'\s]+["\']?', r'\1***', text) return text @@ -277,30 +267,6 @@ def call_llm(repo: str, number: int, pr: dict[str, Any], diff: str, truncated: b if not api_url or not api_key: print("Noema LLM review unavailable: NOEMA_LLM_API_URL or NOEMA_LLM_API_KEY is not configured.") return None - parsed = urllib.parse.urlparse(api_url) - if parsed.scheme.lower() not in {"http", "https"}: - raise ValueError("URL scheme must be http or https") - hostname = (parsed.hostname or "").lower() - if not hostname: - raise ValueError("URL must have a valid hostname") - if hostname in {"localhost", "localhost.localdomain"} or hostname.endswith(".localhost"): - raise ValueError("URL cannot target localhost") - try: - addrinfo = socket.getaddrinfo(hostname, None) - except socket.gaierror: - pass - else: - for result in addrinfo: - ip_str = result[4][0] - try: - ip = ipaddress.ip_address(ip_str) - except ValueError: - continue - if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_unspecified: - raise ValueError("URL cannot target internal IP addresses") - - if not (api_url.startswith("http://") or api_url.startswith("https://")): - raise ValueError(f"NOEMA_LLM_API_URL must start with http:// or https:// to prevent SSRF vulnerabilities, got: {api_url}") prompt = { "role": "user", @@ -329,6 +295,9 @@ def call_llm(repo: str, number: int, pr: dict[str, Any], diff: str, truncated: b prompt, ], } + if not (api_url.startswith("http://") or api_url.startswith("https://")): + raise ValueError(f"URL must start with http:// or https://, got: {api_url}") + request = urllib.request.Request( api_url, data=json.dumps(payload).encode("utf-8"), diff --git a/scripts/ci/opencode_review_normalize_output.py b/scripts/ci/opencode_review_normalize_output.py index 88b93187..c7cbeb63 100755 --- a/scripts/ci/opencode_review_normalize_output.py +++ b/scripts/ci/opencode_review_normalize_output.py @@ -36,6 +36,15 @@ "could not access required evidence", "evidence was truncated", "truncated evidence", + "no changes detected", + "no changes were detected", + "no changes found", + "no changes were found", + "no files or changes were found", + "no files or changes found", + "no actionable changes to review", + "no changes to review", + "no changed files", ) STRUCTURAL_FAILURE_PATTERNS = ( @@ -395,9 +404,6 @@ def contradicts_material_changed_file_scope(reason: str, summary: str) -> bool: def mentions_actual_changed_file(reason: str, summary: str) -> bool: """Return whether an approval names an exact current-head changed file.""" changed_files = current_changed_files() - combined = f"{reason}\n{summary}".casefold() - if not changed_files and ("no executable changes" in combined or "no changed files" in combined or "no changes" in combined or "no ui codebase changes" in combined): - return True if not changed_files: return mentions_changed_file_evidence(reason, summary) combined = f"{reason}\n{summary}" @@ -407,9 +413,6 @@ def mentions_actual_changed_file(reason: str, summary: str) -> bool: def mentions_verification_posture(reason: str, summary: str) -> bool: """Return whether an approval records the concrete review surfaces checked.""" combined = f"{reason}\n{summary}".casefold() - if not current_changed_files() and ("no executable changes" in combined or "no changed files" in combined or "no changes" in combined or "no ui codebase changes" in combined): - # Handle no-op PRs with empty/no changed files where deep verification labels may be omitted by model. - return True return ( all(label in combined for label in APPROVAL_VERIFICATION_LABELS) and "codegraph" in combined @@ -478,8 +481,6 @@ def coverage_section_is_valid(section: str) -> bool: def mentions_full_coverage(reason: str, summary: str) -> bool: """Return whether test and docstring coverage labels cite valid evidence.""" combined = f"{reason}\n{summary}".casefold() - if not current_changed_files() and ("no executable changes" in combined or "no changed files" in combined or "no changes" in combined or "no ui codebase changes" in combined): - return True coverage_section = label_section(combined, "coverage:") docstring_section = label_section(combined, "docstring coverage:") required_sections = (coverage_section, docstring_section) diff --git a/scripts/ci/opencode_review_prompt_template.md b/scripts/ci/opencode_review_prompt_template.md index 4888b89a..2e1c444b 100644 --- a/scripts/ci/opencode_review_prompt_template.md +++ b/scripts/ci/opencode_review_prompt_template.md @@ -12,8 +12,6 @@ Review by positive evidence, not by absence of known blockers. APPROVE is valid Find bugs. Compare the PR title, body, linked issue context, and actual diff, then inspect the connected code paths, rendering path, tests, docs, generated artifacts, deployment/operation paths, and previous behavior that the changed code now interacts with. Do not review the changed hunk as an isolated island: look for contradictions between the PR intent and repository code, between docs and code, between API/schema names and consumers, between UI rendering and state/data flow, between tests and implementation, and between generated files and their source of truth. If the PR promises files, tests, docs, migrations, generated artifacts, contracts, or behavior that are absent, request changes. Also infer missing files from source evidence: new imports without implementation, new routes without tests/docs, schema changes without migration/rollback, API or CLI behavior without contract tests, generated artifact sources without regenerated outputs, docs claims without code support, config changes without examples, and workflow/tooling changes without self-tests. When a required file is missing, anchor the finding to the closest changed reference, manifest, test, workflow, route, import, docs claim, or generated-artifact contract and explain exactly which file/artifact must be added or updated. Check correctness, edge cases, error paths, API compatibility, auth/authz, tenant isolation, secrets, privacy, data integrity, concurrency, migrations, deployment/rollback, observability, performance, resource use, dependency license and supply-chain risk, IaC/cloud/Docker behavior, package/build/test/lint/security contracts, repository conventions, accessibility, i18n/l10n, developer experience, and user experience. Check naming and reserved-word safety for every changed database object, table, column, primary key, foreign key, index, constraint, API field, event name, configuration key, route, class, function, method, file path, generated model, and serialized contract. Prefer the repository's existing convention, but require names to be specific, non-reserved, and meaningfully composed: avoid bare `id`, `name`, `type`, `value`, `data`, `user`, `order`, `group`, `key`, or SQL/platform reserved words when a two-word snake_case, camelCase, PascalCase, or local equivalent such as `order_item_id`, `projectId`, or `UserProfile` would be clearer and safer. For database primary keys, foreign keys, join tables, migrations, and generated ORM models, compare nearby schema conventions and flag ambiguous single-word identifiers or reserved words that can cause query, ORM, serialization, or cross-database portability bugs. At the start of review, define the UX and DX surfaces for this PR from evidence. UX surfaces may include web UI, CLI behavior, API responses, SDK/library contracts, generated files, docs, logs, error messages, workflow/status-check output, review comments, configuration, operator runbooks, onboarding/setup, and migration paths. DX surfaces may include local setup, scripts, tests, lint/coverage/security commands, CI reliability, error diagnostics, review feedback quality, package/release contracts, observability for maintainers, code readability, extension points, and conventions. If a surface is absent, name the closest affected human or automation interaction instead of writing "not applicable." For breaking changes, use git history and deployment evidence when available to discuss bridge modules, migration paths, rollout/rollback, and lower-version compatibility. -Identifier exposure and enumeration safety is a security blocker, not a style note: when a primary key or any identifier exposed in an API response, URL path or query, redirect, filename, cache key, or other client-visible surface is a sequential or auto-incrementing integer (SERIAL/BIGSERIAL, AUTO_INCREMENT, IDENTITY, or an ORM auto-increment id), return REQUEST_CHANGES because sequential ids let attackers enumerate and reach other records (IDOR/enumeration — the Coupang breach exploited guessable sequential ids); require a non-sequential, non-guessable identifier at every exposed boundary such as a random UUIDv4 or random token, treat time-ordered ULID/UUIDv7 as acceptable only when creation-order leakage is harmless, and accept an internal-only auto-increment key solely when it is never exposed and a separate opaque identifier is used at every external boundary, treating unclear exposure as exposed. Require every newly added or renamed identifier — tables, columns, keys, indexes, constraints, API fields, event names, config keys, routes, classes, functions, methods, variables, files, generated models, and serialized contracts — to be composed of two or more meaningful words rather than a bare single word or reserved word, in the idiomatic case of that file's language (snake_case for Python/Ruby/Rust/SQL and DB columns, camelCase for JavaScript/TypeScript/Java/Kotlin/Swift members, PascalCase for types/classes and Go exported names, SCREAMING_SNAKE_CASE for constants), following the repository's existing convention where it differs and never forcing one language's casing onto another; a single-word or reserved name such as id, data, user, type, value, run, handler, or temp is a blocker when a two-word equivalent such as order_item_id, projectId, UserProfile, or parseRequest is clearer and safer, while short-lived loop indices and idiomatic single-letter math variables are exempt. - For numerical programming, scientific programming, statistical modeling, simulation, optimization, signal processing, ML metrics, estimators, inference code, or formula-heavy implementations, obtain the original paper, specification, vignette, or authoritative reference through web_search/webfetch or official documentation before approving. Verify that formulas, constants, likelihoods, priors, gradients, convergence criteria, random seeds, tolerances, parameter constraints, and numerical stability tricks match the source or are explicitly justified. Require repo-native or scratch PoC evidence that the implementation recovers true parameters on known synthetic data, including skewed or ill-conditioned true-parameter regimes when the method claims robustness; compare against baseline or prior behavior when available. Strengthen the test case set before approving: do not accept a single happy-path test for one function when the scientific claim depends on multiple regimes. Add augmented scratch tests or require repository tests for balanced and skewed parameters, boundary values, degeneracy/zero-variance inputs, random-seed determinism, numerical tolerance, convergence failure, and prior-version or published-example parity as appropriate, then execute the relevant repository test command or sandboxed PoC. Lack of a host toolchain is not a reason to skip execution: provision an isolated Docker, Docker Compose, devcontainer, Nix, or temporary package-install sandbox and run the augmented verification there with no production credentials or persistent repository mutation. If an LLM or patch changes an equation, estimator, loss, distribution, or statistic without source-backed derivation and regression tests that would catch parameter-recovery failure, request changes. For web application surfaces, execute or cite repository-native frontend/backend/E2E evidence when available. Use Playwright when the repository supports it, and review both behavior and rendering: desktop plus one mobile viewport when practical, visual screenshot or toHaveScreenshot evidence, DOM locator assertions using data-testid/role/label selectors, ARIA snapshot or accessibility-tree evidence for changed interactive surfaces when practical, console error/warn collection, failed network request collection, and target-flow interaction proof. If backend and frontend services exist, prefer python3 scripts/ci/sandboxed_web_e2e.py --repo-root "$OPENCODE_SOURCE_WORKDIR" ... and cite SANDBOXED_WEB_E2E_RESULT. For non-web or unsupported surfaces, state the exact missing contract rather than treating partial execution as full evidence. diff --git a/tests/test_codeql_pr_workflow_contract.py b/tests/test_codeql_pr_workflow_contract.py deleted file mode 100644 index 5833691d..00000000 --- a/tests/test_codeql_pr_workflow_contract.py +++ /dev/null @@ -1,24 +0,0 @@ -from pathlib import Path - - -REPO_ROOT = Path(__file__).resolve().parents[1] - - -def test_codeql_pr_workflow_uploads_head_and_merge_sarif_for_ruleset_gate() -> None: - workflow = (REPO_ROOT / ".github/workflows/codeql-pr.yml").read_text( - encoding="utf-8" - ) - - assert "name: CodeQL PR" in workflow - assert "branches: [main, master, develop]" in workflow - assert "upload: always" in workflow - assert "detect-languages:" in workflow - assert "analyze-head:" in workflow - assert "analyze-merge:" in workflow - assert "merge_commit_sha != ''" in workflow - assert "CodeQL merge preview" in workflow - assert "github.event.pull_request.head.sha" in workflow - assert "github.event.pull_request.merge_commit_sha" in workflow - assert "refs/pull/{0}/head" in workflow - assert "refs/pull/{0}/merge" in workflow - assert "security-events: write" in workflow \ No newline at end of file diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index cc68ff28..3a366666 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -60,12 +60,6 @@ def test_scrub_sensitive_data(): assert noema.scrub_sensitive_data("password: xyz") == "password: ***" -def test_scrub_sensitive_data_authorization_headers(): - assert noema.scrub_sensitive_data("Authorization: Basic dXNlcjpwYXNz") == "Authorization: Basic ***" - assert noema.scrub_sensitive_data("Proxy-Authorization: Basic dXNlcjpwYXNz") == "Proxy-Authorization: Basic ***" - assert noema.scrub_sensitive_data("authorization: bearer xyz") == "authorization: bearer ***" - - def test_split_repo_and_graphql(monkeypatch): with pytest.raises(ValueError): noema.split_repo("owner") @@ -204,11 +198,6 @@ def test_call_llm_handles_configuration_and_verdicts(monkeypatch): monkeypatch.delenv("NOEMA_LLM_API_KEY", raising=False) assert noema.call_llm("owner/repo", 1, pr, "diff", False) is None - monkeypatch.setenv("NOEMA_LLM_API_URL", "file:///etc/passwd") - monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret") - with pytest.raises(ValueError, match="must start with http:// or https://"): - noema.call_llm("owner/repo", 1, pr, "diff", False) - monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat") monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret") monkeypatch.setenv("NOEMA_LLM_MODEL", "review-model") @@ -233,61 +222,6 @@ def fake_urlopen(request, timeout): with pytest.raises(RuntimeError, match="unsupported decision"): noema.call_llm("owner/repo", 1, pr, "diff", False) - # Test case-insensitive valid URL - monkeypatch.setenv("NOEMA_LLM_API_URL", "HTTPS://llm.example.test/chat") - monkeypatch.setattr(noema.urllib.request, "urlopen", fake_urlopen) - assert noema.call_llm("owner/repo", 1, pr, "diff", True)["decision"] == "approve" - - # Test invalid scheme (and no original URL in error) - monkeypatch.setenv("NOEMA_LLM_API_URL", "file:///etc/passwd") - with pytest.raises(ValueError, match="URL scheme must be http or https"): - noema.call_llm("owner/repo", 1, pr, "diff", False) - - # Test localhost rejection - monkeypatch.setenv("NOEMA_LLM_API_URL", "http://localhost/chat") - with pytest.raises(ValueError, match="URL cannot target localhost"): - noema.call_llm("owner/repo", 1, pr, "diff", False) - - # Test missing hostname - monkeypatch.setenv("NOEMA_LLM_API_URL", "http:///chat") - with pytest.raises(ValueError, match="URL must have a valid hostname"): - noema.call_llm("owner/repo", 1, pr, "diff", False) - - # Test internal IP rejection - monkeypatch.setenv("NOEMA_LLM_API_URL", "http://169.254.169.254/chat") - with pytest.raises(ValueError, match="URL cannot target internal IP addresses"): - noema.call_llm("owner/repo", 1, pr, "diff", False) - - import socket - original_getaddrinfo = socket.getaddrinfo - - # Test DNS resolution bypass - monkeypatch.setenv("NOEMA_LLM_API_URL", "http://resolved-to-local.example.com/chat") - def fake_getaddrinfo(host, port, *args, **kwargs): - if host == "resolved-to-local.example.com": - return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", ("127.0.0.1", 0))] - return original_getaddrinfo(host, port, *args, **kwargs) - monkeypatch.setattr(socket, "getaddrinfo", fake_getaddrinfo) - with pytest.raises(ValueError, match="URL cannot target internal IP addresses"): - noema.call_llm("owner/repo", 1, pr, "diff", False) - - # Test unresolved hostname does not break - monkeypatch.setenv("NOEMA_LLM_API_URL", "http://unresolved.example.com/chat") - def fake_getaddrinfo_error(host, port, *args, **kwargs): - raise socket.gaierror("Name or service not known") - monkeypatch.setattr(socket, "getaddrinfo", fake_getaddrinfo_error) - monkeypatch.setattr(noema.urllib.request, "urlopen", fake_urlopen) - assert noema.call_llm("owner/repo", 1, pr, "diff", True)["decision"] == "approve" - - # Test invalid IP string from getaddrinfo (unlikely but theoretically possible) - monkeypatch.setenv("NOEMA_LLM_API_URL", "http://weird-dns.example.com/chat") - def fake_getaddrinfo_invalid_ip(host, port, *args, **kwargs): - if host == "weird-dns.example.com": - return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", ("not_an_ip", 0))] - return original_getaddrinfo(host, port, *args, **kwargs) - monkeypatch.setattr(socket, "getaddrinfo", fake_getaddrinfo_invalid_ip) - assert noema.call_llm("owner/repo", 1, pr, "diff", True)["decision"] == "approve" - def test_format_findings_and_submit_review(monkeypatch): findings = noema.format_findings( @@ -371,3 +305,11 @@ def test_parse_args_and_main(monkeypatch): with pytest.raises(SystemExit, match="--pr-number must be positive"): noema.main(["--repo", "owner/repo", "--pr-number", "0"]) + +def test_call_llm_rejects_unsafe_url_schemes(monkeypatch): + pr = make_pr() + monkeypatch.setenv("NOEMA_LLM_API_URL", "file:///etc/passwd") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret") + + with pytest.raises(ValueError, match="URL must start with http:// or https://"): + noema.call_llm("owner/repo", 1, pr, "diff", False) diff --git a/tests/test_opencode_review_normalize_output.py b/tests/test_opencode_review_normalize_output.py index fc317101..fcc7d37d 100644 --- a/tests/test_opencode_review_normalize_output.py +++ b/tests/test_opencode_review_normalize_output.py @@ -108,19 +108,6 @@ def test_actual_changed_file_detection_prefers_current_head_file_list(tmp_path, ) monkeypatch.setenv("OPENCODE_CHANGED_FILES_FILE", str(changed_files)) - monkeypatch.delenv("OPENCODE_CHANGED_FILES_FILE", raising=False) - assert norm.mentions_actual_changed_file("No executable changes here", "no changed files") - assert norm.mentions_verification_posture("No executable changes here", "no changed files") - assert norm.mentions_full_coverage("No executable changes here", "no changed files") - assert norm.mentions_actual_changed_file("No changes", "no changes") - assert norm.mentions_verification_posture("No changes", "no changes") - assert norm.mentions_full_coverage("No changes", "no changes") - assert norm.mentions_actual_changed_file("No UI codebase changes", "No UI codebase changes") - assert norm.mentions_verification_posture("No UI codebase changes", "No UI codebase changes") - assert norm.mentions_full_coverage("No UI codebase changes", "No UI codebase changes") - monkeypatch.setenv("OPENCODE_CHANGED_FILES_FILE", str(changed_files)) - - assert norm.current_changed_files() == { ".github/workflows/opencode-review.yml", "scripts/ci/opencode_review_normalize_output.py",