From dd12539a2c0f5863f2bbb3f2efd5d50e645f46e1 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Sun, 5 Jul 2026 20:04:49 -0300 Subject: [PATCH] fix(api): Zod-validate POST /api/github-skills + document new gate envs + pin merge-integrity actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three latent heavy-CI reds surfaced by the VPS validation dispatch (the fast path never runs these gates): - t06 route-validation: POST /api/github-skills destructured request.json() blind — a non-array 'targets' would .map-crash. Now validateBody(zod) with defaults preserved (Hard Rule #7). Guard: tests/unit/github-skills-route-validation.test.ts (4/4). - env-doc-sync: document OMNIROUTE_SKIP_SYSTEM_TRUST (#6310) and the changelog-integrity gate envs CHANGELOG_BASE_REF/ALLOW_CHANGELOG_REMOVALS (#6300) in .env.example + ENVIRONMENT.md. - zizmor ratchet: the new merge-integrity job's checkout/setup-node uses were unpinned (+1 finding, 160 > baseline 159); pinned by SHA -> 158 (< baseline). --- .env.example | 10 +++++ .github/workflows/quality.yml | 4 +- docs/reference/ENVIRONMENT.md | 3 ++ src/app/api/github-skills/route.ts | 23 ++++++----- .../github-skills-route-validation.test.ts | 38 +++++++++++++++++++ 5 files changed, 64 insertions(+), 14 deletions(-) create mode 100644 tests/unit/github-skills-route-validation.test.ts diff --git a/.env.example b/.env.example index a17c036260..c741ba39c6 100644 --- a/.env.example +++ b/.env.example @@ -1725,6 +1725,16 @@ APP_LOG_TO_FILE=true # where the operator trusts the CA manually (e.g. via Node's extra-CA-certs mechanism). # OMNIROUTE_NO_SUDO=0 +# ── Test/CI-only guards (never needed in production) ── +# Set automatically by tests/_setup/isolateDataDir.ts and the CI workflows: the +# test suite must NEVER mutate the OS trust store (a fake test PEM installed via +# update-ca-certificates broke all system TLS on a persistent runner, 2026-07-05). +# OMNIROUTE_SKIP_SYSTEM_TRUST=1 +# check-changelog-integrity.mjs (anti CHANGELOG-eat gate): explicit base ref +# override, and the justified-removal escape hatch for intentional bullet removals. +# CHANGELOG_BASE_REF=origin/release/v0.0.0 +# ALLOW_CHANGELOG_REMOVALS=1 + # ── 1Proxy egress pool ── # Used by: src/lib/oneproxySync.ts — fetches proxy nodes from the OmniRoute # CrofAI 1Proxy service. Disable, override URL, or tune the import quality. diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 1adfdd0230..fda787cfb9 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -195,11 +195,11 @@ jobs: API_KEY_SECRET: ci-lint-api-key-secret-long DISABLE_SQLITE_AUTO_BACKUP: "true" steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 with: fetch-depth: 0 persist-credentials: false - - uses: actions/setup-node@v6 + - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6 with: node-version: ${{ env.CI_NODE_VERSION }} cache: npm diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index 565a4c26c1..1ca37d603f 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -1014,6 +1014,9 @@ Provider quota endpoints, network tunnels (Tailscale, Ngrok, MITM debug proxy), | `MITM_IDLE_TIMEOUT_MS` | `60000` | `src/mitm/socketTimeouts.ts`, `src/mitm/server.cjs` | Idle socket timeout (ms) for proxied connections; idle sockets past this are torn down to avoid leaking half-open tunnels. | | `MITM_VERBOSE` | `1` | `src/mitm/server.cjs`, `src/mitm/_internal/bypass.cjs` | Routing-decision log verbosity: `0` silences, higher values log more bypass/route decisions. | | `OMNIROUTE_NO_SUDO` | `0` | `src/mitm/systemCommands.ts` | Set `1` (truthy) to strip the leading `sudo` from MITM cert-trust commands — for root-less / user-namespaced deployments where the operator trusts the CA manually (e.g. via Node's extra-CA-certs mechanism). | +| `OMNIROUTE_SKIP_SYSTEM_TRUST` | `0` | `src/mitm/cert/install.ts`, `src/mitm/tproxy/caTrust.ts` | Test/CI-only guard: set `1` to make cert trust install/uninstall a no-op so the suite never mutates the OS trust store. Set automatically by the test setup and CI workflows. | +| `CHANGELOG_BASE_REF` | _(auto)_ | `scripts/check/check-changelog-integrity.mjs` | Explicit base ref for the anti CHANGELOG-eat gate (defaults to the PR base branch in CI, or the highest `release/v*`). | +| `ALLOW_CHANGELOG_REMOVALS` | `0` | `scripts/check/check-changelog-integrity.mjs` | Set `1` to turn intentional CHANGELOG bullet removals into a report instead of a failure (justify in the PR body). | | `ONEPROXY_ENABLED` | `true` | `src/lib/oneproxySync.ts` | Enable the 1Proxy egress pool sync. | | `ONEPROXY_API_URL` | `https://1proxy-api.aitradepulse.com` | `src/lib/oneproxySync.ts` | 1Proxy service API URL override. | | `ONEPROXY_MAX_PROXIES` | `500` | `src/lib/oneproxySync.ts` | Maximum proxies imported per sync. | diff --git a/src/app/api/github-skills/route.ts b/src/app/api/github-skills/route.ts index 303d92568b..f4687a3847 100644 --- a/src/app/api/github-skills/route.ts +++ b/src/app/api/github-skills/route.ts @@ -10,10 +10,18 @@ * Body: { repoName, targets, description } */ import { NextRequest, NextResponse } from "next/server"; +import { z } from "zod"; import { searchGitHubSkills } from "@/lib/skills/githubCollector"; import { matchesSearch } from "@/shared/utils/turkishText"; +import { validateBody } from "@/shared/validation/helpers"; import { buildErrorBody, sanitizeErrorMessage } from "@omniroute/open-sse/utils/error"; +const installSkillSchema = z.object({ + repoName: z.string().min(1), + targets: z.array(z.string().min(1)).optional().default(["hermes"]), + description: z.string().optional().default(""), +}); + export const dynamic = "force-dynamic"; export async function GET(request: NextRequest) { @@ -57,20 +65,11 @@ export async function GET(request: NextRequest) { export async function POST(request: NextRequest) { try { - const body = await request.json(); - const { - repoName, - targets = ["hermes"], - description = "", - } = body as { - repoName?: string; - targets?: string[]; - description?: string; - }; - - if (!repoName || typeof repoName !== "string") { + const parsed = validateBody(installSkillSchema, await request.json()); + if (!parsed.success) { return NextResponse.json(buildErrorBody(400, "repoName is required"), { status: 400 }); } + const { repoName, targets, description } = parsed.data; const { resolveInstallPath, INSTALL_TARGETS } = await import("@/lib/skills/githubCollector"); const skillName = repoName.split("/").pop() || repoName; diff --git a/tests/unit/github-skills-route-validation.test.ts b/tests/unit/github-skills-route-validation.test.ts new file mode 100644 index 0000000000..3cd859b6b3 --- /dev/null +++ b/tests/unit/github-skills-route-validation.test.ts @@ -0,0 +1,38 @@ +// t06 route-validation: POST /api/github-skills must validate its body with a +// Zod schema (Hard Rule #7) instead of blind request.json() destructuring. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +const { POST } = await import("../../src/app/api/github-skills/route.ts"); + +function post(body: unknown): Request { + return new Request("http://localhost/api/github-skills", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +test("POST github-skills: missing repoName → 400", async () => { + const res = await POST(post({ targets: ["hermes"] }) as never); + assert.equal(res.status, 400); +}); + +test("POST github-skills: non-array targets → 400 (was silently .map-crashing before Zod)", async () => { + const res = await POST(post({ repoName: "a/b", targets: "hermes" }) as never); + assert.equal(res.status, 400); +}); + +test("POST github-skills: valid body → 200 with per-target results and defaults applied", async () => { + const res = await POST(post({ repoName: "owner/skill-repo" }) as never); + assert.equal(res.status, 200); + const body = (await res.json()) as { skillName: string; results: { target: string }[] }; + assert.equal(body.skillName, "skill-repo"); + assert.equal(body.results[0].target, "hermes"); +}); + +test("POST github-skills: 400 error body does not leak stack traces", async () => { + const res = await POST(post({}) as never); + const text = await res.text(); + assert.ok(!text.includes("at /"), "stack trace leaked"); +});