Pular para conteúdo

AUR-521: trilha de auditoria e SARIF para o code scanning da org

Contrato

Qualquer aurumcode review (--base ou --pr) pode escrever dois arquivos adicionais, cada um só quando a flag correspondente é dada (zero flags = comportamento de hoje, byte a byte):

  • --auditoria <arquivo>: um registro JSON (internal/render.AuditRecord) com o digest da política, o SHA do workflow, o repositório, o SHA revisado, o modelo, o veredito, a decisão do gate (pass/fail/ inconclusive + motivo), os achados que efetivamente reprovaram o gate (blocking_findings), as exceções aplicadas (exceptions_applied, sempre uma lista — vazia até a AUR-520 passar a preenchê-la) e a cobertura (completa ou não, com os arquivos que ficaram de fora).
  • --sarif <arquivo>: um documento SARIF 2.1.0 (internal/render. BuildSARIFLog/WriteSARIF) com tool.driver.rules (um por rule_id citado, com o título da seção de skill quando dinâmico), results com ruleId, level, message, location (URI relativo ao repositório + startLine) e partialFingerprints com a impressão digital canônica do achado. Uma revisão inconclusiva ainda produz um SARIF estruturalmente válido: invocations[0].executionSuccessful=false e um toolExecutionNotifications nomeando o motivo (AC-004).

Ver docs/configuration.md's "Trilha de auditoria e SARIF" section para a sintaxe completa.

A identidade canônica de achado (compartilhada com a AUR-494)

internal/render.FindingFingerprint (internal/render/finding_identity.go) é a ÚNICA definição de "mesmo achado" neste projeto. Ela existe antes da AUR-494 chegar de propósito: o card pede explicitamente que a impressão digital seja "a mesma identidade de findings da AUR-494", então esta entrega a define uma vez, documentada como canônica, para que a AUR-494 a importe em vez de redefinir a sua própria.

A identidade é RuleID + caminho normalizado + linha + contexto de código normalizado (sha256, hex). Nunca inclui:

  • o texto livre do modelo isoladamente (message) — o modelo reformula a mesma explicação de execução para execução, o que faria a impressão digital variar sem o achado ter mudado;
  • qualquer valor por chamada (relógio, nonce, PID) — exatamente o que MUT-001 ataca: uma impressão digital que muda a cada rodada para o MESMO achado quebra toda deduplicação/rastreamento por impressão digital (SARIF incluso).

normalizeFindingPath/normalizeFindingContext tornam a identidade estável sob reformatação inofensiva (barra/backslash, ./, espaços) sem esconder uma mudança real de conteúdo.

Crucialmente, o Context usado pela identidade NUNCA vem de issue.Evidence/issue.Message (campos escritos pelo modelo e reescritos pelo motor de revisão, redactLinesKeepingMarkers, antes de chegar aqui) — render.FindingIdentityFor(diff, issue, filter) (internal/render/finding_identity_diff.go) lê a linha real do hunk do PRÓPRIO diff revisado em (issue.File, issue.Line, issue.Side). Uma revisão independente deste card confirmou por teste decisivo (TestAUR521AuditFingerprintStableAcrossTwoRuns, duas rodadas do MESMO diff com message/evidence diferentes) que a versão inicial — que usava issue.Evidence — produzia impressões digitais diferentes para o mesmo achado sempre que o texto do modelo mudava, o que é exatamente o defeito que este card existe para evitar. O Context passa por filter.Redact antes de alimentar o hash: um segredo literal na linha revisada não pode fazer o MESMO achado gerar impressões digitais diferentes só porque o segredo foi rotacionado entre duas execuções.

Decisões de design

  • A decisão do gate tem UMA fonte, e evaluateGate é ela. Uma revisão independente apontou que a primeira versão desta entrega relia num matchedBlockingFindings que RE-DERIVAVA o loop de limiar em cmd/aurumcode/aur521.go — uma segunda implementação da mesma decisão, com risco real de divergir da primeira (e, de fato, da AUR-520, que chegou com seu próprio loop de exceção dentro do MESMO loop). Corrigido: evaluateGate (cmd/aurumcode/policygate.go) agora retorna gateDecision.BlockingFindings e gateDecision.AppliedExceptions ([]render.AuditFinding/[]render.AuditException) preenchidos NO EXATO ponto em que Breach/a exceção ativa já são decididos — nunca em um segundo loop. matchedBlockingFindings foi removido por completo; writeComplianceArtifacts só lê in.gate.BlockingFindings/ AppliedExceptions. A mesma lista de exceções aplicadas também indexa quais achados o SARIF marca como suppressions (achado excepado nunca aparece em blocking_findings E aparece suprimido no SARIF, com a mesma justificativa — testado de ponta a ponta, TestAUR521ExceptedFindingSuppressedNotBlocking). auditGateOutcome nomeia a decisão (pass/fail/inconclusive) a partir de gateDecision MAIS o motivo de inconclusivo já calculado pelo chamador (gateInconclusiveReason), porque uma execução pode ser inconclusiva com NENHUM gate declarado (gate: ausente do config) — e, nesse caso, o SARIF da mesma execução já marca executionSuccessful=false; os dois arquivos têm que concordar (testado, TestAUR521AuditAndSARIFAgreeOnInconclusive).
  • AppliedExceptions só é gravado achado por achado DENTRO do loop de limiar — e esse loop só roda quando gate.fail_on está declarado. evaluateGate checa a exceção de cada achado dentro do MESMO for que o limiar de severidade usa; gate.Threshold() retorna ok=false (e a função volta antes de entrar no for) quando a política não declarou fail_on nenhum, mesmo que gate.fail_on esteja ausente mas exceptions/inconclusive estejam presentes. Nesse caso AppliedExceptions fica vazio mesmo que uma exceção real cobrisse um achado do diff — a exceção simplesmente nunca é avaliada, porque o gate em si nunca chega a avaliar achado nenhum. Isso é consistente com o próprio desenho da AUR-520 (uma exceção só importa onde há um limiar para ela evitar), mas significa que exceptions_applied não é "toda exceção configurada que bateria com o diff" — é "toda exceção que o gate, rodando de verdade, realmente aplicou".
  • SARIF: redigir o Path ANTES de normalizeFindingPath, não depois. Defeito real encontrado numa terceira revisão: normalizeFindingPath troca toda barra invertida literal por barra normal antes de WriteSARIF redigir o documento inteiro (redactSARIFLog) — então um segredo com uma barra invertida dentro do Path já perdeu a forma exata que o filtro tinha registrado (a barra invertida) no momento em que a redação roda, e sobrevive, com barras normais, dentro de artifactLocation.uri. Corrigido com uma passada extra, redactSARIFFindings, que redige cada campo do achado (Path incluso) ANTES de BuildSARIFLog sequer normalizar o caminho — WriteSARIF agora tem três passadas de redação (achado crú, documento construído, texto final serializado), cada uma pegando uma forma que as outras não pegam. Testado (TestAUR521SARIFRedactsSecretWithBackslashInPath).
  • Redação ANTES de serializar, mais a passada final no sink. Outro defeito real da primeira versão: redigir só o JSON já serializado (filter.Redact(string(data))) não pega um segredo cujo ", \ ou quebra de linha embutida o json.Marshal escapou para uma sequência de bytes diferente da que os regexes de redação casam contra o valor CRU do segredo. redactAuditRecord/redactSARIFLog (internal/render/audit.go/sarif.go) agora redigem cada campo string individualmente ANTES de json.MarshalIndent — construindo slices/ structs novos, nunca mutando o valor do chamador — e a passada final sobre o texto completo continua depois, como defesa em profundidade (nunca removida). Testado com três formas de segredo escapável (", \, quebra de linha) em ambos os arquivos (TestAUR521WriteAuditRecordRedactsEscapedSecrets, TestAUR521SARIFRedactsEscapedSecrets).
  • exceptions_applied sempre presente, agora POPULADO pela AUR-520 real. BuildAuditRecord nunca o deixa nil/omitido; com a AUR-520 já no main mesclado nesta entrega, o campo deixou de ser "vazio até a AUR-520" e passou a carregar a exceção real que evaluateGate aplicou (dono, motivo, validade), e a mesma exceção aparece suprimida no SARIF.
  • omitted_files hoje é a união de IgnoredPaths e FilteredPaths (reviewCoverageBreakdown, cmd/aurumcode/passes.go) — os arquivos config-ocultados e os binários/grandes demais, ambos já rastreados por caminho. Omissões por orçamento de tokens (Budget) só chegam como contagem (code_files_omitted), nunca por caminho, porque o construtor de prompt nunca nomeia esses arquivos de volta paracmd/aurumcode` — uma lacuna pré-existente deste card, não uma introduzida por ele.
  • model é --modelo/opts.modelo, com LLM_MODEL como alternativa (firstNonEmpty) — .github/workflows/review.yml nunca passa --modelo, só a env LLM_MODEL; sem essa alternativa, model sairia sempre vazio em todo run de CI real. Ainda não é o nome resolvido pelo PROVEDOR (que pode escolher o padrão do endpoint quando os dois ficam vazios) — documentado como melhor esforço: o card não exige uma segunda chamada ao provedor só para nomear o modelo de volta.
  • workflow_sha lê só AURUMCODE_WORKFLOW_SHA, sem alternativa para GITHUB_SHA. GITHUB_SHA é reservado pelo GitHub Actions num evento pull_request (o merge commit sintético; ver o comentário de pr.go ~linha 58/996, AUR-504) e um env: de step não consegue sobrescrevê-lo — o mesmo é esperado para GITHUB_WORKFLOW_SHA, embora isso não tenha sido confirmado por execução real (offline). Por isso o workflow reutilizável reenvia o SHA do próprio workflow sob um nome que a plataforma não reserva (AURUMCODE_WORKFLOW_SHA: ${{ job.workflow_sha }}). A versão anterior desta entrega caía para GITHUB_SHA quando AURUMCODE_WORKFLOW_SHA estava vazio — mas GITHUB_SHA já tem seu próprio campo (reviewed_sha); essa alternativa fazia workflow_sha silenciosamente DUPLICAR o SHA revisado sempre que a CLI corresse fora deste workflow (um --base local, por exemplo), escondendo que a versão do workflow é simplesmente desconhecida. Corrigido: sem AURUMCODE_WORKFLOW_SHA, workflow_sha sai vazio, nunca igual a reviewed_sha por acaso.
  • action.yml não foi tocado. action.yml invoca o binário por argumentos posicionais fixos que scripts/action-entrypoint.sh consome — e esse script está fora dos paths do card (só em read_paths). Adicionar --auditoria/--sarif por esse caminho exigiria editar o entrypoint; a trilha de auditoria/SARIF é, por desenho do card ("a política de segurança... vem embutida no workflow reutilizável exigido pela org"), uma responsabilidade do workflow reutilizável, não da Action Docker direta. .github/workflows/review.yml já chama o binário com uma lista de argumentos explícita (não posicional) e foi estendido ali.
  • diffLineAt (internal/render/finding_identity_diff.go) trata a marca \ No newline at end of file como qualquer outra linha. Um defeito real: o default: do switch tratava QUALQUER linha que não começasse com +/- como uma linha de contexto (' '), avançando os dois contadores de linha — incluindo a própria marca "sem newline no final do arquivo" do git, que não é uma linha de diff nenhuma. O resultado era deslocar por um todo número de linha depois dela, mapeando um achado para o CONTEÚDO ERRADO (ou para nada). Corrigido para tratar ' ' explicitamente (exatamente como internal/review/scope.go já faz no loop equivalente) e um default: que não avança nenhum contador — a mesma convenção, nunca uma segunda. Testado (TestAUR521DiffLineAtSkipsNoNewlineMarker).

Upload para o code scanning: NUNCA dentro do workflow reutilizável

A entrega anterior deste card fazia .github/workflows/review.yml chamar github/codeql-action/upload-sarif ele mesmo, com security-events: write no permissions: do próprio job. Uma revisão independente apontou o defeito real disso: uma reusable workflow NÃO consegue conceder a si mesma uma permissão que o CALLER não já tenha — GitHub recusa iniciar a chamada inteira (não só o passo do upload) quando o workflow_call declara uma permissão que o token do caller não tem. Como nenhum caller existente (code-review.yml deste repositório, os exemplos, qualquer outro repositório que já use este workflow) concedia security-events: write, essa versão teria quebrado TODO caller no primeiro merge — não um upload que falha com 403, o job de review inteiro nunca rodando.

Corrigido: review.yml nunca declara security-events: write e nunca chama upload-sarif. Os dois arquivos saem como artefatos simples do job (actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a, v7.0.1 — a única action que este card ainda chama diretamente): aurumcode-sarif-<PR> e aurumcode-audit-<PR>, ambos só quando o arquivo correspondente não está vazio (o passo Check compliance artifacts are non-empty, if: always(), testa -s antes — os dois arquivos são pré-criados com : > só para o bind mount do Docker não criar um diretório no lugar, e um upload nunca roda sobre um arquivo de 0 bytes) e com if: always() nos dois uploads (um gate reprovado falha o passo "Run and publish code review" — exatamente o caso em que o artefato mais importa; sem always() ele seria pulado justo ali).

Quem quer o SARIF no code scanning de verdade roda um SEGUNDO job, no seu próprio workflow — onde conceder security-events: write a si mesmo é normal, sem cruzar fronteira de reusable workflow — que baixa o artefato e chama github/codeql-action/upload-sarif@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2 (v4.38.2, SHA resolvido pelo coordenador via API do GitHub) com a category fixa aurumcode-policy-gate (estável de propósito: um upload --pr, ou um futuro run agendado, atualiza a MESMA análise no code scanning em vez de acumular um conjunto de alertas que nunca é limpo). O job exato está documentado em docs/configuration.md's "Trilha de auditoria e SARIF" section; code-review.yml deste próprio repositório ainda não tem esse segundo job (fora dos paths da AUR-521, não criado por este card) — até que alguém o adicione, o SARIF deste repositório fica disponível como artefato do job de review, mas não chega ao code scanning.

Outras lacunas conhecidas (declaradas, não corrigidas pelo prazo)

  • AC-004 no --pr, só quando o gate chega a rodar. Uma falha de provedor/transporte (llm.ErrAllProvidersFailed, llm.ErrBudgetExceeded) retorna de runPRReview ANTES do bloco do gate — e, portanto, antes de writeComplianceArtifacts — exatamente como o comentário de gateInconclusiveReason em pr.go já documenta para o próprio gate ("NOTE: a genuine provider/transport failure... returns earlier in this function, before the gate is ever reached"). Nessa saída antecipada específica, com --auditoria/--sarif dados, o workflow fica com os dois arquivos vazios (pré-criados por : > em review.yml) em vez de um registro/SARIF marcando inconclusivo. O caminho --base (runReview) não tem essa lacuna: toda saída antecipada equivalente já vira gateInconclusiveReason = "provider_failure" antes do ponto onde este card escreve. Corrigir no --pr exigiria uma segunda chamada a writeComplianceArtifacts nesses retornos antecipados especificamente; é acompanhado pela AUR-537. Não corrigido aqui.

Fechado nesta rodada: o canário de AC-005 agora também é testado na LINHA DO DIFF (TestAUR521RedactsSecretCanaryFromDiffLine), não só no texto do modelo — o caso que importa depois da mudança para render.FindingIdentityFor, cujo Context vem do diff. Um achado cujo message/evidence NUNCA mencionam o canário, mas cuja linha revisada o contém, ainda não deixa o canário alcançar nenhum dos dois arquivos.

Testes

  • internal/render/finding_identity_test.go, internal/render/audit_test.go, internal/render/sarif_test.go: unidade, isolando cada escritor do gate/motor de revisão.
  • cmd/aurumcode/aur521_test.go: comportamento real, através de runReview (--base) e runPRReview (--pr), no mesmo estilo de aur519_e2e_test.go.
  • tests/acceptance/AUR-521.sh: seletores all, AC-001..AC-005, AC-002-MUT-001 (funde um nonce de relógio na impressão digital; TestAUR521FindingFingerprintStableAcrossRuns deve virar RED), seletor desconhecido sai 64, falha de infraestrutura sai 79.