Skip to content

security: harden scanning, signing keyring, MCP and the eval protocol - #80

Open
criptogus wants to merge 7 commits into
mainfrom
security/harden-scanning-keyring-mcp-and-eval-protocol
Open

criptogus wants to merge 7 commits into
mainfrom
security/harden-scanning-keyring-mcp-and-eval-protocol

Conversation

@criptogus

Copy link
Copy Markdown
Owner

Por que

Auditoria de segurança do próprio repo (o pitch do produto é segurança) + estudo dos dois projetos oficiais da NVIDIA — SkillEvaluator e SkillSpector — para trazer o que eles fazem de melhor.

O que muda

1. Scanning (CI)

  • SkillSpector pinado por SHA no commit exato da v2.13.2 (antes: main). Instalação via git fetch <sha> + checkout FETCH_HEAD.
  • Secret scanning bloqueante (gitleaks) em PR, push na main e cron semanal de histórico completo, com a action pinada por SHA.
  • .gitleaks.toml com allowlists auditadas: primeiro scan → 60 achados → 0 segredos reais (JWTs role:anon do Supabase que já vão no bundle, actor IDs da Apify, hashes SHA-256 de tokenizer, fixtures synthetic_*). Cada entrada documenta o porquê.

2. Keyring de assinatura (bug crítico)

O ring de produção estava vazio ("keys": []) e, com isso, verifyRelease() aceitava qualquer chave pública — "verified by SuperAgentSkill" era forjável.

  • Chave Ed25519 registrada via o fluxo existente (manage-signing-keys.ts rotate); privada fora do repo, 0600.
  • Assinatura/verificação fail-closed: ring sem chave ativa não assina nem verifica; download degrada para unsigned_reason explícito; chave fora do ring é rejeitada.
  • scripts/check-signing-keyrings.ts bloqueante no CI (exatamente 1 chave ativa por ring, fingerprint confere) + runbook docs/SIGNING-KEYS.md.

3. Protocolo de eval (SkillEvaluator)

  • metric_set_version + score_policy_version em todo benchmark pareado.
  • Verdict: PASS ≥ +5pp, FAIL ≤ −10pp, LIMITED no meio, INCONCLUSIVE quando os braços são indistinguíveis (a skill nunca ativou) ou a suíte tem < 5 casos — nunca PASS de graça.
  • basis: ledger por case com EXCLUDED_ONLY_IN_BASELINE, EXCLUDED_ONLY_IN_CERTIFIED, EXCLUDED_JUDGE_MIXED.
  • Judge identity registrada por braço; juízes mistos ⇒ INCONCLUSIVE.
  • Evidence stamp (src/lib/adversarial/stamping.ts): full_suite, holdout_included, scan_clean (o MESMO gate bloqueante), judge_source_recorded. Run amostrado não auto-certifica.
  • Trust score: política versionada, evidência com stamp não-certificável é retratada (fallback ao prior), e scan incompleto limita o score a 0.69 e bloqueia verified — análise incompleta nunca fica verde (nos dois scorers, score.ts e scoring.ts).

4. Golden patterns do SkillSpector no gate

  • P2 instruções escondidas em Unicode TAG (U+E0000–U+E007F), MP1–MP3 memory poisoning (seed de credencial = critical), TP3 parameter-description injection, AR1–AR3 anti-refusal, SC9 bytecode .pyc/.pyo.
  • Audit extraído para src/lib/security/package-audit.ts (uma implementação para o gate CI e runtime); scripts/audit-skills.ts vira só o CLI.
  • Golden tests + casos benignos para conter falso positivo. Conteúdo do repo continua limpo (0 bloqueados).

5. MCP

  • Primeira suíte de contrato do endpoint (tests/mcp-server.test.ts): handshake, catálogo único com inputSchema, tool grátis anônima, write anônima → 401 com id ecoado, bearer forjado fail-closed com X-MCP-Auth, GET info + CORS. Loader de alias @/ em node puro.
  • security_scan: scanner como guardrail de runtime, grátis/anônimo, roda o MESMO gate do publish e devolve safe_to_install/verdict + evidência por finding (campo/offset/excerpt), com limites de análise explícitos.

6. Rug-pull detection

  • src/lib/marketplace/rug-pull.ts + scripts/check-rug-pull.ts: mesmo slug+versão com conteúdo ou tool surface diferente = rug pull (crítico); publisher trocado e rollback de versão também. Release legítima (versão nova) não é finding.

Verificação

  • npm run test:all → 410/410 pass, 0 fail
  • npx tsc --noEmit → limpo
  • gitleaks detect → no leaks found
  • audit:skills → 0 pacotes bloqueados
  • check-signing-keyrings → ambos os rings ok

⚠️ Ação necessária antes do merge/deploy

Os secrets SIGNING_PRIVATE_KEY / SIGNING_PUBLIC_KEY (GitHub + .env) precisam ser o par registrado no ring (id 25de98852a1efe47), senão as assinaturas de plugin/trust degradam para unsigned (fail-closed) até a rotação. Os PEMs estão em ~/.super-agent-skill/keys/.

Gustavo Caetano added 7 commits October 31, 2026 18:49
…ning

- Pin NVIDIA SkillSpector to the exact commit of v2.13.2 (fetched by SHA, not
  tag/main) and install it from that ref — a moving upstream can no longer
  weaken the advisory gate silently.
- Add a BLOCKING gitleaks workflow (PR diff, push-to-main, weekly full-history
  cron), with the action pinned by SHA like SkillSpector.
- Add .gitleaks.toml allowlists audited against the first scan: 60 findings,
  0 real secrets (public Supabase anon JWTs shipped in client bundles, Apify
  actor IDs, tokenizer SHA-256 hashes, synthetic_* fixtures). Every entry
  documents WHY it is safe to expose.
The production ring shipped with "keys": [] — and with an empty ring both
signRelease() and verifyRelease() operated anchor-less, so ANY public key
passed verification. "Verified by SuperAgentSkill" was forgeable.

- Register the produced Ed25519 key (id 25de98852a1efe47) via the existing
  manage-signing-keys.ts flow (private half stays outside the repo, 0600).
- signRelease/verifyRelease and plugin signing now refuse to operate without
  an active key in the deployment ring; plugin downloads degrade to an
  explicit unsigned_reason instead of emitting an orphan signature.
- Keep ring injectable for tests/BYO deployments (additive optional arg).
- Add scripts/check-signing-keyrings.ts (CI-blocking hygiene: exactly one
  active key per ring, fingerprint match) and docs/SIGNING-KEYS.md runbook
  for rotation/revocation/offline verification.
- Tests cover: foreign-key forgery rejected, empty ring is an error not a
  pass, and env keys outside the ring sign nothing.
Imports the SkillEvaluator protocol so a benchmark can no longer grade noise:

- pairedComparison now emits metric_set_version + score_policy_version, a
  verdict (PASS >= +5pp, FAIL <= -10pp, LIMITED in between, INCONCLUSIVE when
  the arms are indistinguishable or the suite is under 5 cases) and a per-case
  basis ledger (EXCLUDED_ONLY_IN_BASELINE / ONLY_IN_CERTIFIED / JUDGE_MIXED).
  "Skill never activated" is a first-class INCONCLUSIVE, never a free PASS.
- Judge identity is recorded per arm; mixing judge models degenerates to
  INCONCLUSIVE instead of silently comparing two experiments.

- New src/lib/adversarial/stamping.ts: an eval run carries an evidence stamp
  with the required checks (full_suite, holdout_included, scan_clean,
  judge_source_recorded). eval-adversarial.ts writes judge_model and the stamp
  (scan_clean comes from the SAME blocking audit gate); a sampled run can no
  longer self-certify.

- Trust score (both scorers) gains a versioned policy, retracts adversarial
  evidence when the stamp is present but not certify-worthy, and caps the
  score at 0.69 when the security scan did not complete — an incomplete
  analysis can never render green, and never verifies.
- Extract the package audit into src/lib/security/package-audit.ts so the CI
  gate and runtime callers share ONE implementation (scripts/audit-skills.ts
  is now only the file-walking CLI; rejectAt is an option).
- Add NVIDIA SkillSpector golden patterns to the production guard:
  P2 hidden Unicode TAG instructions (U+E0000..U+E007F), MP1-MP3 memory
  poisoning (credential seeding is critical), TP3 parameter-description
  smuggling, AR1-AR3 unconditional anti-refusal overrides.
- Add golden pattern SC9 (.pyc/.pyo bytecode) to the malicious-function rules.
- Sanitized output now strips the tag block too; the AR rule is scoped to
  unconditional refusal wipes so benign guidance ("never refuse outright")
  does not false-positive.
- Golden test cases pin each pattern plus benign look-alikes; the repo's own
  content still audits clean (0 packages blocked).
Port of SkillSpector's --mcp-registry-compare idea to the registry: a package
that keeps its identity (slug + version + publisher) while its content or tool
surface changes is the rug-pull attack, and version strings alone cannot see
it.

- src/lib/marketplace/rug-pull.ts: pure comparator over published vs candidate
  snapshots — CONTENT_CHANGED_SAME_VERSION (critical), TOOL_SET_CHANGED,
  PUBLISHER_CHANGED, VERSION_ROLLBACK. A new version with new content is a
  release, not a finding.
- scripts/check-rug-pull.ts: compares a published manifest against
  content/skills (or --write-manifest to snapshot at release time); exit 1 on
  findings with --fail-on-findings.
- Tests cover the attack, the legitimate release, order-insensitive tool sets
  and deterministic content hashing.
…rver

The MCP endpoint is the product's most public contract and had zero coverage;
src/lib/mcp/tools/skills.ts alone is ~2.8k lines.

- tests/mcp-server.test.ts: hermetic end-to-end contract tests (no network, no
  Supabase) — initialize handshake, unique/complete tools/list with
  inputSchema, free tools anonymous, anonymous write rejected with a matched
  JSON-RPC id, forged bearer fail-closed with X-MCP-Auth diagnostics, GET info
  + CORS, and security_scan behaviour. A tiny alias loader teaches plain node
  the tsconfig "@/" path and extensionless relative imports.
- src/lib/mcp/tools/security.ts: security_scan runs the SAME blocking gate as
  publish time (package-audit) and returns safe_to_install, verdict, worst
  severity and per-finding evidence (field/offset/excerpt), with explicit
  analysis-scope limits — a clean scan means 'nothing detected', not 'proven
  safe'. Free and anonymous: safety is not paywalled.
- Register it in the server and FREE_TOOLS; add --test-force-exit to the test
  scripts because MCP streams remain open by design, plus npm scripts for the
  new suites and check:rug-pull.
…rift

- check-signing-keyrings.ts now uses node:crypto/node:fs only: it runs in the
  gitleaks job, which has no node_modules, so importing the zod-based keyring
  parser failed there. Same checks (spec, 16-char id == spki fingerprint,
  statuses, exactly one active key, revoked keys need a reason).
- Regenerate cli/super-agent.js: the CLI bundles the prompt-injection guard,
  so the new SkillSpector golden patterns are part of its output (--check was
  correctly refusing the stale bundle).
- Prettier drift in four pre-existing files (payments.functions, pricing-copy,
  routes/pricing, its test) broke the lint gate before this branch; formatting
  only, no behaviour change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant