A pull request has one Dependency review comment, changed in place, and the review can be run again on demand. Every head move posted the review's summary as a new comment, so catching flagon-io/g1t#39 up with main left a second, identical one beside the first. The security service now finds g1t's own comment on the pull request (written by g1t, starting with the review's heading, so pull requests from before this are found too) and edits it through work's edit_comment when the review says something new, leaves it alone when it says the same, and adds one only for a review with dependency changes to show and none there yet. A head whose lockfile changes are the ones already reviewed is not reviewed again: the review row keeps a key of each lockfile's added and removed lines (not where they sit, which moves when the base is merged in) and the policy, and a push with the same key, such as g1t's catch-up merge commit, takes the earlier result as it is, setting the same status on the new head with no OSV query and no comment. A comparison that is truncated or leaves a lockfile's lines out has no key and is reviewed. Since advisories are published every day, the review can be made again: rerun_dependency_review (security RPC; API POST /repos/{owner}/{name}/pulls/{number}/dependency-review/rerun, MCP security rerun_dependency_review, security:write, the Write role, on an open pull request) reviews the head again, sets the status and changes the same comment; the pull request's checks show Re-run dependency review beside g1t's Dependency review status, and the Security checks page has Re-run. The supply chain guide and the MCP reference say so; the OpenAPI document, its example and the CLI's operation table are regenerated.
- Checked:
- cargo test for g1t-security (92, new: the review key survives the base merged in and moves with a lockfile change or the policy, the key kept in the row, one comment found and changed only when the review says something new), g1t-scan (66, new: every summary starts with the heading), g1t-contracts (307) and g1t-api (204, the OpenAPI document regenerated); G1T_WRITE_OPS=1 cargo test -p g1t ops_table; cargo check --target wasm32-unknown-unknown for g1t-security and g1t-api; the site's typecheck (wrangler types, react-router typegen, tsc -b --force).
20 files+192−164 files not counted0/20 viewed
| 260 | 260 | Op::Security(SecurityOp::GetDependencyGraph), | |
| 261 | 261 | Op::Security(SecurityOp::GetSbom), | |
| 262 | 262 | Op::Security(SecurityOp::CompareDependencies), | |
| 263 | + | Op::Security(SecurityOp::RerunDependencyReview), | |
| 263 | 264 | ], | |
| 264 | 265 | ), | |
| 265 | 266 | ( |
| 713 | 713 | } | |
| 714 | 714 | ||
| 715 | 715 | impl Op { | |
| 716 | − | pub const ALL: [Op; 378] = [ | |
| 716 | + | pub const ALL: [Op; 379] = [ | |
| 717 | 717 | Op::Whoami, | |
| 718 | 718 | Op::GetWorkspace, | |
| 719 | 719 | Op::CreateWorkspace, | |
| ⋯ | |||
| 924 | 924 | Op::Security(SecurityOp::GetDependencyGraph), | |
| 925 | 925 | Op::Security(SecurityOp::GetSbom), | |
| 926 | 926 | Op::Security(SecurityOp::CompareDependencies), | |
| 927 | + | Op::Security(SecurityOp::RerunDependencyReview), | |
| 927 | 928 | Op::Security(SecurityOp::GetSettings), | |
| 928 | 929 | Op::Security(SecurityOp::UpdateSettings), | |
| 929 | 930 | Op::Security(SecurityOp::GetWorkspaceSettings), | |
| 8609 | 8609 | "deny_licenses": [] | |
| 8610 | 8610 | } | |
| 8611 | 8611 | }, | |
| 8612 | + | "rerun_dependency_review": { | |
| 8613 | + | "params": { | |
| 8614 | + | "owner": "flagon-io", | |
| 8615 | + | "name": "hello", | |
| 8616 | + | "number": 12 | |
| 8617 | + | }, | |
| 8618 | + | "response": { | |
| 8619 | + | "base": "4b1d7c0e9a2f6e3d8c5b0a7f4e1d2c3b4a5f6e7d", | |
| 8620 | + | "head": "9f2c4e1a7b3d5f60812a4c6e8b0d2f4a6c8e0b13", | |
| 8621 | + | "changes": [ | |
| 8622 | + | { | |
| 8623 | + | "change_type": "added", | |
| 8624 | + | "manifest": "package-lock.json", | |
| 8625 | + | "ecosystem": "npm", | |
| 8626 | + | "name": "lodash", | |
| 8627 | + | "version": "4.17.21", | |
| 8628 | + | "relationship": "direct", | |
| 8629 | + | "development": false, | |
| 8630 | + | "license": "MIT", | |
| 8631 | + | "purl": "pkg:npm/lodash@4.17.21", | |
| 8632 | + | "vulnerabilities": [], | |
| 8633 | + | "denied_license": false, | |
| 8634 | + | "failing": false | |
| 8635 | + | }, | |
| 8636 | + | { | |
| 8637 | + | "change_type": "removed", | |
| 8638 | + | "manifest": "package-lock.json", | |
| 8639 | + | "ecosystem": "npm", | |
| 8640 | + | "name": "lodash", | |
| 8641 | + | "version": "4.17.20", | |
| 8642 | + | "relationship": "direct", | |
| 8643 | + | "development": false, | |
| 8644 | + | "license": "MIT", | |
| 8645 | + | "purl": "pkg:npm/lodash@4.17.20", | |
| 8646 | + | "vulnerabilities": [], | |
| 8647 | + | "denied_license": false, | |
| 8648 | + | "failing": false | |
| 8649 | + | } | |
| 8650 | + | ], | |
| 8651 | + | "passed": true, | |
| 8652 | + | "headline": "2 dependency changes, none vulnerable", | |
| 8653 | + | "fail_on": "high", | |
| 8654 | + | "deny_licenses": [] | |
| 8655 | + | }, | |
| 8656 | + | "notes": "The pull request's one dependency review comment is changed in place; nothing new is posted when the review says the same." | |
| 8657 | + | }, | |
| 8612 | 8658 | "get_security_settings": { | |
| 8613 | 8659 | "params": { | |
| 8614 | 8660 | "owner": "flagon-io", |
| 332 | 332 | route("GET", "/repos/:owner/:name/dependency-graph", Op::Security(SecurityOp::GetDependencyGraph), &[]), | |
| 333 | 333 | route("GET", "/repos/:owner/:name/dependency-graph/sbom", Op::Security(SecurityOp::GetSbom), &[]), | |
| 334 | 334 | route("GET", "/repos/:owner/:name/dependency-graph/compare/:basehead", Op::Security(SecurityOp::CompareDependencies), &[]), | |
| 335 | + | route("POST", "/repos/:owner/:name/pulls/:number/dependency-review/rerun", Op::Security(SecurityOp::RerunDependencyReview), &[]), | |
| 335 | 336 | route("GET", "/repos/:owner/:name/security/settings", Op::Security(SecurityOp::GetSettings), &[]), | |
| 336 | 337 | route("PATCH", "/repos/:owner/:name/security/settings", Op::Security(SecurityOp::UpdateSettings), &[]), | |
| 337 | 338 | route("GET", "/workspaces/:workspace/security/settings", Op::Security(SecurityOp::GetWorkspaceSettings), &[]), |
| 54 | 54 | GetDependencyGraph, | |
| 55 | 55 | GetSbom, | |
| 56 | 56 | CompareDependencies, | |
| 57 | + | RerunDependencyReview, | |
| 57 | 58 | GetSettings, | |
| 58 | 59 | UpdateSettings, | |
| 59 | 60 | GetWorkspaceSettings, | |
| ⋯ | |||
| 73 | 74 | /// Every one: `Op::ALL` lists each as `Op::Security(…)`, which a test | |
| 74 | 75 | /// checks against this. | |
| 75 | 76 | #[cfg(test)] | |
| 76 | − | pub const ALL: [SecurityOp; 38] = [ | |
| 77 | + | pub const ALL: [SecurityOp; 39] = [ | |
| 77 | 78 | SecurityOp::ListSecretAlerts, | |
| 78 | 79 | SecurityOp::GetSecretAlert, | |
| 79 | 80 | SecurityOp::UpdateSecretAlert, | |
| ⋯ | |||
| 99 | 100 | SecurityOp::GetDependencyGraph, | |
| 100 | 101 | SecurityOp::GetSbom, | |
| 101 | 102 | SecurityOp::CompareDependencies, | |
| 103 | + | SecurityOp::RerunDependencyReview, | |
| 102 | 104 | SecurityOp::GetSettings, | |
| 103 | 105 | SecurityOp::UpdateSettings, | |
| 104 | 106 | SecurityOp::GetWorkspaceSettings, | |
| ⋯ | |||
| 141 | 143 | SecurityOp::GetDependencyGraph => "get_dependency_graph", | |
| 142 | 144 | SecurityOp::GetSbom => "get_sbom", | |
| 143 | 145 | SecurityOp::CompareDependencies => "compare_dependencies", | |
| 146 | + | SecurityOp::RerunDependencyReview => "rerun_dependency_review", | |
| 144 | 147 | SecurityOp::GetSettings => "get_security_settings", | |
| 145 | 148 | SecurityOp::UpdateSettings => "update_security_settings", | |
| 146 | 149 | SecurityOp::GetWorkspaceSettings => "get_workspace_security_settings", | |
| ⋯ | |||
| 185 | 188 | SecurityOp::GetDependencyGraph => "Get the dependency graph", | |
| 186 | 189 | SecurityOp::GetSbom => "Export an SBOM", | |
| 187 | 190 | SecurityOp::CompareDependencies => "Compare dependencies", | |
| 191 | + | SecurityOp::RerunDependencyReview => "Re-run dependency review", | |
| 188 | 192 | SecurityOp::GetSettings => "Get a repository's security settings", | |
| 189 | 193 | SecurityOp::UpdateSettings => "Update a repository's security settings", | |
| 190 | 194 | SecurityOp::GetWorkspaceSettings => "Get a workspace's security settings", | |
| ⋯ | |||
| 228 | 232 | SecurityOp::GetDependencyGraph => "Get the dependency graph: every package the lockfiles on the default branch resolve, per lockfile, with whether it is direct or transitive (where the lockfile says), for development, its license when recorded, its package URL and its open vulnerability alerts.", | |
| 229 | 233 | SecurityOp::GetSbom => "Export the dependency graph as an SPDX 2.3 JSON document, in sbom. Every package is named by its package URL.", | |
| 230 | 234 | SecurityOp::CompareDependencies => "Compare the dependencies at two commits, branches or tags (basehead, as base...head): what was added and removed per lockfile, with the known vulnerabilities of what was added and whether it passes the repository's dependency review policy. Needs the g1t plan on a private repository.", | |
| 235 | + | SecurityOp::RerunDependencyReview => "Run the Dependency review check again on an open pull request's head, with the advisories known now: its status is set again and its one comment on the pull request is changed in place (or added, when it has dependency changes to show and none is there yet). Returns the review. g1t reviews a pull request by itself when it opens and when its head moves, carrying the result over when the head's lockfile changes are the ones reviewed before. Takes the Write role. Needs the g1t plan on a private repository.", | |
| 231 | 236 | SecurityOp::GetSettings => "Get a repository's security settings: when the Code scanning check fails, dependency review and its policy, whether secrets in the history open alerts, whether it takes private vulnerability reports, its workspace's settings, and whether the paid features are on for it.", | |
| 232 | 237 | SecurityOp::UpdateSettings => "Change a repository's security settings: code_scanning_gate (none, errors, critical, high, medium or any), dependency_review, review_fail_on (critical, high, medium, low or none), review_deny_licenses (SPDX ids), review_comment, history_alerts (whether secrets found in the history open alerts; push protection stays on) and private_vulnerability_reporting. Takes the Admin role, or a security manager of the workspace. Require the Code scanning and Dependency review checks in branch protection to gate merges on them.", | |
| 233 | 238 | SecurityOp::GetWorkspaceSettings => "Get a workspace's security settings (delegated bypass, validity checks) and whether it has Security and quality, which comes with the g1t plan.", | |
| ⋯ | |||
| 428 | 433 | &["repo", "id", "state"], | |
| 429 | 434 | ), | |
| 430 | 435 | SecurityOp::GetDependencyGraph | SecurityOp::GetSbom | SecurityOp::GetSettings => (json!({ "repo": repo() }), &["repo"]), | |
| 436 | + | SecurityOp::RerunDependencyReview => ( | |
| 437 | + | json!({ "repo": repo(), "number": { "type": "integer", "description": "The pull request's number." } }), | |
| 438 | + | &["repo", "number"], | |
| 439 | + | ), | |
| 431 | 440 | SecurityOp::CompareDependencies => ( | |
| 432 | 441 | json!({ | |
| 433 | 442 | "repo": repo(), | |
| ⋯ | |||
| 1037 | 1046 | }; | |
| 1038 | 1047 | pass(services, "dependency_review", &DependencyReviewArgs { viewer: viewer.clone(), repo, base, head }).await | |
| 1039 | 1048 | } | |
| 1049 | + | SecurityOp::RerunDependencyReview => { | |
| 1050 | + | let Some(repo) = repo else { return failed(FailureCode::Invalid, &need_repo()) }; | |
| 1051 | + | let Some(number) = whole(input, "number").filter(|number| *number > 0) else { | |
| 1052 | + | return failed(FailureCode::Invalid, "number is the pull request's number."); | |
| 1053 | + | }; | |
| 1054 | + | pass(services, "rerun_dependency_review", &PullScanningArgs { viewer: viewer.clone(), repo, number }).await | |
| 1055 | + | } | |
| 1040 | 1056 | SecurityOp::GetSettings => { | |
| 1041 | 1057 | let Some(repo) = repo else { return failed(FailureCode::Invalid, &need_repo()) }; | |
| 1042 | 1058 | pass(services, "security_settings", &SecuritySettingsArgs { viewer: viewer.clone(), repo }).await | |
| ⋯ | |||
| 1241 | 1257 | } | |
| 1242 | 1258 | assert_eq!(scope_for("get_sbom"), Some(Scope::SecurityRead)); | |
| 1243 | 1259 | assert_eq!(scope_for("upload_sarif"), Some(Scope::SecurityWrite)); | |
| 1260 | + | assert_eq!(scope_for("rerun_dependency_review"), Some(Scope::SecurityWrite)); | |
| 1244 | 1261 | // No run decides about security. | |
| 1245 | 1262 | for name in ["bypass_push_protection", "review_bypass_request", "update_security_settings"] { | |
| 1246 | 1263 | assert!(g1t_contracts::credentials::NEVER.contains(&name), "{name}"); | |
| 483 | 483 | a("dependency_graph", Op::Security(SecurityOp::GetDependencyGraph), "Every package the lockfiles resolve, direct or transitive"), | |
| 484 | 484 | a("sbom", Op::Security(SecurityOp::GetSbom), "The dependency graph as an SPDX 2.3 document"), | |
| 485 | 485 | a("compare_dependencies", Op::Security(SecurityOp::CompareDependencies), "What changes in dependencies between base...head"), | |
| 486 | + | a("rerun_dependency_review", Op::Security(SecurityOp::RerunDependencyReview), "Run a pull request's Dependency review check again"), | |
| 486 | 487 | a("settings", Op::Security(SecurityOp::GetSettings), "A repository's security settings"), | |
| 487 | 488 | a("update_settings", Op::Security(SecurityOp::UpdateSettings), "Change when checks fail and dependency review's policy"), | |
| 488 | 489 | a("workspace_settings", Op::Security(SecurityOp::GetWorkspaceSettings), "A workspace's delegated bypass and validity checks"), |
| 80 | 80 | `g1t.sh/<owner>/<project>/security/pulls/<number>`. Require **Dependency | |
| 81 | 81 | review** under **Settings → Branches** to block merges on it. | |
| 82 | 82 | ||
| 83 | + | A pull request has one review comment. When a later review says something | |
| 84 | + | new, that comment is changed in place; when it says the same, the comment | |
| 85 | + | is left alone. A push whose lockfile changes are the ones already reviewed, | |
| 86 | + | such as catching the branch up with its base, is not reviewed again: the | |
| 87 | + | check's result carries over to the new head. | |
| 88 | + | ||
| 89 | + | Advisories are published every day, so a review can be made again: **Re-run | |
| 90 | + | dependency review** in the pull request's checks, or **Re-run** on its | |
| 91 | + | Security checks page, takes the Write role. It updates the check and the | |
| 92 | + | same comment. | |
| 93 | + | ||
| 83 | 94 | In the repository's **Security settings**: | |
| 84 | 95 | ||
| 85 | 96 | | Setting | Default | | | |
| ⋯ | |||
| 115 | 126 | | `GET /repos/{owner}/{name}/dependency-graph` | The graph, per lockfile. | `security:read` | | |
| 116 | 127 | | `GET /repos/{owner}/{name}/dependency-graph/sbom` | The SPDX document, in `sbom`. | `security:read` | | |
| 117 | 128 | | `GET /repos/{owner}/{name}/dependency-graph/compare/{basehead}` | What changed between two commits, and whether it passes review. | `security:read` | | |
| 129 | + | | `POST /repos/{owner}/{name}/pulls/{number}/dependency-review/rerun` | Runs a pull request's Dependency review check again and returns the review. | `security:write` | | |
| 118 | 130 | | `GET /repos/{owner}/{name}/vulnerability-alerts` | Lists alerts; filter with `state`, `severity`, `ecosystem`, `package`. | `security:read` | | |
| 119 | 131 | | `GET /workspaces/{workspace}/vulnerability-alerts` | The same across a workspace. | `security:read` | | |
| 120 | 132 | | `GET /repos/{owner}/{name}/vulnerability-alerts/{id}` | One alert. | `security:read` | | |
| 121 | 133 | | `PATCH /repos/{owner}/{name}/vulnerability-alerts/{id}` | `state` `dismissed` with `reason` and `comment`, or `open`. | `security:write` | | |
| 122 | 134 | ||
| 123 | 135 | Over MCP, the [`security` tool](/reference/mcp/#security) has | |
| 124 | − | `dependency_graph`, `sbom`, `compare_dependencies`, `vulnerability_alerts`, | |
| 136 | + | `dependency_graph`, `sbom`, `compare_dependencies`, `rerun_dependency_review`, `vulnerability_alerts`, | |
| 125 | 137 | `vulnerability_alert` and `update_vulnerability_alert`. Webhooks: | |
| 126 | 138 | `vulnerability_alert.created`, `.fixed`, `.dismissed` and `.reopened`. | |
| 591 | 591 | | [`dependency_graph`](/reference/api/supply-chain/get-dependency-graph/) | Every package the lockfiles resolve, direct or transitive, with licenses. | `repo` | `security:read` | | |
| 592 | 592 | | [`sbom`](/reference/api/supply-chain/get-sbom/) | The dependency graph as an SPDX 2.3 document, in `sbom`. | `repo` | `security:read` | | |
| 593 | 593 | | [`compare_dependencies`](/reference/api/supply-chain/compare-dependencies/) | What changes between `basehead` (`base...head`), and whether it passes dependency review. | `repo`, `basehead` | `security:read` | | |
| 594 | + | | [`rerun_dependency_review`](/reference/api/supply-chain/rerun-dependency-review/) | Runs a pull request's Dependency review check again; its one comment changes in place. Write role. | `repo`, `number` | `security:write` | | |
| 594 | 595 | | [`settings`](/reference/api/security-settings/get-security-settings/) | A repository's security settings, and whether the paid features are on. | `repo` | `security:read` | | |
| 595 | 596 | | [`update_settings`](/reference/api/security-settings/update-security-settings/) | Change `code_scanning_gate`, `dependency_review`, `review_fail_on`, `review_deny_licenses`, `review_comment`. Admin role, or a security manager. | `repo` | `security:write` | | |
| 596 | 597 | | [`workspace_settings`](/reference/api/security-settings/get-workspace-security-settings/) | A workspace's delegated bypass and validity checks. | `workspace` | `security:read` | |
Binary or large file; its contents are not shown.
| 21 | 21 | import { useMemo, useState } from "react"; | |
| 22 | 22 | import { Form, Link } from "react-router"; | |
| 23 | 23 | ||
| 24 | − | import type { CheckResult, CheckRun, CommitStatus, Job, Mergeable, Pull, RequiredCheck } from "@g1t/contracts"; | |
| 24 | + | import { | |
| 25 | + | type CheckResult, | |
| 26 | + | type CheckRun, | |
| 27 | + | type CommitStatus, | |
| 28 | + | DEPENDENCY_REVIEW_CHECK, | |
| 29 | + | type Job, | |
| 30 | + | type Mergeable, | |
| 31 | + | type Pull, | |
| 32 | + | type RequiredCheck, | |
| 33 | + | } from "@g1t/contracts"; | |
| 25 | 34 | ||
| 26 | 35 | import { useAddresses } from "../lib/addresses"; | |
| 27 | 36 | import { duration } from "./actions"; | |
| ⋯ | |||
| 406 | 415 | pull.fork ? `/${pull.fork.namespace}/${pull.fork.name}/commit/${sha}` : `${base}/commit/${sha}`; | |
| 407 | 416 | const older = run != null && pull.headCommit != null && run.headCommit !== pull.headCommit; | |
| 408 | 417 | const failedRuns = statuses.filter((status) => statusStanding(status) === "failed" && runIdOf(status)); | |
| 418 | + | // g1t's own Dependency review check, which anyone who may start runs can | |
| 419 | + | // have made again (after an advisory is published, say). | |
| 420 | + | const dependencyReview = statuses.some((status) => status.context === DEPENDENCY_REVIEW_CHECK && status.source === "security"); | |
| 409 | 421 | ||
| 410 | 422 | return ( | |
| 411 | 423 | <div aria-busy={loading || undefined}> | |
| ⋯ | |||
| 458 | 470 | </Hint> | |
| 459 | 471 | </Form> | |
| 460 | 472 | ))} | |
| 473 | + | {canRerunWorkflows && dependencyReview && !settled && ( | |
| 474 | + | <Form method="post"> | |
| 475 | + | <input type="hidden" name="action" value="rerun-dependency-review" /> | |
| 476 | + | <Hint label="Review its dependency changes again, with the advisories known now"> | |
| 477 | + | <SubmitButton variant="outline" match={{ action: "rerun-dependency-review" }} pending="Re-running…"> | |
| 478 | + | <RotateCw size={13} /> | |
| 479 | + | Re-run dependency review | |
| 480 | + | </SubmitButton> | |
| 481 | + | </Hint> | |
| 482 | + | </Form> | |
| 483 | + | )} | |
| 461 | 484 | </div> | |
| 462 | 485 | </div> | |
| 463 | 486 | {error && ( | |
| 85 | 85 | import { ViolationList } from "../../components/rules"; | |
| 86 | 86 | import { PullCodeOwnersPanel, TeamReviewer } from "../../components/codeowners"; | |
| 87 | 87 | import { notFound } from "../../lib/not-found.server"; | |
| 88 | − | import { actions, deployments, identity, inbox, repos, work } from "../../lib/services.server"; | |
| 88 | + | import { actions, deployments, identity, inbox, repos, securitySuite, work } from "../../lib/services.server"; | |
| 89 | 89 | import { commitChecksFor } from "../../lib/commit-checks.server"; | |
| 90 | 90 | import { CommitChecksBadge } from "../../components/commit-checks"; | |
| 91 | 91 | import { DeployLink } from "../../components/deploy"; | |
| ⋯ | |||
| 337 | 337 | const PULL_NEEDS: Record<string, Capability> = { | |
| 338 | 338 | "rerun-workflow": "run", | |
| 339 | 339 | "rerun-failed": "run", | |
| 340 | + | "rerun-dependency-review": "run", | |
| 340 | 341 | merge: "merge", | |
| 341 | 342 | unqueue: "merge", | |
| 342 | 343 | milestone: "triage", | |
| ⋯ | |||
| 401 | 402 | const rerun = await actions.rerun(user, path, String(form.get("run") ?? ""), true); | |
| 402 | 403 | return rerun.ok ? null : { error: rerun.error.message, action }; | |
| 403 | 404 | } | |
| 405 | + | // The Dependency review check, made again on its head: its comment | |
| 406 | + | // changes in place. Who may is the security service's call. | |
| 407 | + | if (action === "rerun-dependency-review") { | |
| 408 | + | const rerun = await securitySuite.rerunDependencyReview(user, path, number); | |
| 409 | + | return rerun.ok ? null : { error: rerun.error.message, action }; | |
| 410 | + | } | |
| 404 | 411 | // Every failed run on its head, run again: Mission control's quick action. | |
| 405 | 412 | if (action === "rerun-failed") { | |
| 406 | 413 | const found = await work.getPull(path, number, user); | |
| ⋯ | |||
| 667 | 674 | canRerunWorkflows={canMerge} | |
| 668 | 675 | settingsUrl={canProtect ? `${base}/settings/branches` : null} | |
| 669 | 676 | conflicting={mergeable === "conflicting" && !noChecks} | |
| 670 | − | error={actionData?.action === "rerun-failed" || actionData?.action === "rerun-workflow" ? actionData.error : null} | |
| 677 | + | error={actionData?.action === "rerun-failed" || actionData?.action === "rerun-workflow" || actionData?.action === "rerun-dependency-review" | |
| 678 | + | ? actionData.error | |
| 679 | + | : null} | |
| 671 | 680 | loading={loading} | |
| 672 | 681 | /> | |
| 673 | 682 | ); | |
| ⋯ | |||
| 1349 | 1358 | </Form> | |
| 1350 | 1359 | )} | |
| 1351 | 1360 | {actionData && | |
| 1352 | − | !["merge", "comment", "rerun-failed", "rerun-workflow", "update", "reviewers", "assign", "edit-comment", "delete-comment"].includes( | |
| 1361 | + | !["merge", "comment", "rerun-failed", "rerun-workflow", "rerun-dependency-review", "update", "reviewers", "assign", "edit-comment", "delete-comment"].includes( | |
| 1353 | 1362 | String(actionData.action), | |
| 1354 | 1363 | ) && <ErrorText>{actionData.error}</ErrorText>} | |
| 1355 | 1364 | </div> | |
| 1 | − | import { ArrowLeft, CircleAlert, CircleCheck } from "lucide-react"; | |
| 2 | − | import { Link } from "react-router"; | |
| 1 | + | import { ArrowLeft, CircleAlert, CircleCheck, RotateCw } from "lucide-react"; | |
| 2 | + | import { Form, Link } from "react-router"; | |
| 3 | 3 | ||
| 4 | 4 | import type { Route } from "./+types/security-pull"; | |
| 5 | 5 | import { page } from "../../lib/meta"; | |
| 6 | 6 | import { SeverityBadge } from "../../components/security"; | |
| 7 | 7 | import { CARD, LIST, SectionHeader, shortRule } from "../../components/security-suite"; | |
| 8 | + | import { ErrorText, SubmitButton } from "../../components/ui"; | |
| 8 | 9 | import { Badge } from "../../components/ui/badge"; | |
| 9 | 10 | import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "../../components/ui/table"; | |
| 10 | 11 | import { cn } from "../../lib/cn"; | |
| 11 | 12 | import { securitySuite } from "../../lib/services.server"; | |
| 12 | − | import { getViewer, requireUser, unwrap } from "../../lib/session.server"; | |
| 13 | − | import { requireInsider } from "../../lib/access.server"; | |
| 13 | + | import { assertSameOrigin, getViewer, requireUser, unwrap } from "../../lib/session.server"; | |
| 14 | + | import { refusal, requireInsider } from "../../lib/access.server"; | |
| 14 | 15 | ||
| 15 | 16 | export function meta({ params, ...args }: Route.MetaArgs) { | |
| 16 | 17 | return page(args, { title: `Security checks · #${params.number} · ${params.owner}/${params.repo} · g1t` }); | |
| ⋯ | |||
| 23 | 24 | return { scanning: unwrap(await securitySuite.pullScanning(repo, Number(params.number) || 0, viewer)) }; | |
| 24 | 25 | } | |
| 25 | 26 | ||
| 27 | + | /** The Dependency review check made again on the pull request's head; its comment changes in place. */ | |
| 28 | + | export async function action({ params, context, request }: Route.ActionArgs) { | |
| 29 | + | assertSameOrigin(request); | |
| 30 | + | const user = requireUser(context, request); | |
| 31 | + | const refused = await refusal(context, params, "run"); | |
| 32 | + | if (refused) return { error: refused }; | |
| 33 | + | const repo = { namespace: params.owner, name: params.repo }; | |
| 34 | + | const rerun = await securitySuite.rerunDependencyReview(user, repo, Number(params.number) || 0); | |
| 35 | + | return rerun.ok ? null : { error: rerun.error.message }; | |
| 36 | + | } | |
| 37 | + | ||
| 26 | 38 | function Verdict({ passed, text }: { passed: boolean; text: string }) { | |
| 27 | 39 | return ( | |
| 28 | 40 | <p className={`flex items-center gap-1.5 text-sm ${passed ? "text-success" : "text-danger"}`}> | |
| ⋯ | |||
| 33 | 45 | } | |
| 34 | 46 | ||
| 35 | 47 | /** What the Code scanning and Dependency review checks found on a pull request: the page their statuses link to. */ | |
| 36 | − | export default function PullSecurity({ loaderData, params }: Route.ComponentProps) { | |
| 48 | + | export default function PullSecurity({ loaderData, actionData, params }: Route.ComponentProps) { | |
| 37 | 49 | const { scanning } = loaderData; | |
| 38 | 50 | const base = `/${params.owner}/${params.repo}`; | |
| 39 | 51 | const onLines = scanning.results.filter((result) => result.new && result.onChangedLine); | |
| ⋯ | |||
| 82 | 94 | )} | |
| 83 | 95 | </section> | |
| 84 | 96 | <section className="space-y-3"> | |
| 85 | − | <h3 className="text-base font-semibold tracking-tight">Dependency review</h3> | |
| 97 | + | <div className="flex flex-wrap items-center justify-between gap-2"> | |
| 98 | + | <h3 className="text-base font-semibold tracking-tight">Dependency review</h3> | |
| 99 | + | <Form method="post"> | |
| 100 | + | <SubmitButton variant="outline" pending="Re-running…"> | |
| 101 | + | <RotateCw size={13} /> | |
| 102 | + | Re-run | |
| 103 | + | </SubmitButton> | |
| 104 | + | </Form> | |
| 105 | + | </div> | |
| 106 | + | {actionData?.error && <ErrorText>{actionData.error}</ErrorText>} | |
| 86 | 107 | {!scanning.review ? ( | |
| 87 | 108 | <p className="text-sm text-muted">Not reviewed yet: it runs when a pull request opens and each time its head moves.</p> | |
| 88 | 109 | ) : ( | |
| 1077 | 1077 | ("get_dependency_graph", Scope::SecurityRead), | |
| 1078 | 1078 | ("get_sbom", Scope::SecurityRead), | |
| 1079 | 1079 | ("compare_dependencies", Scope::SecurityRead), | |
| 1080 | + | ("rerun_dependency_review", Scope::SecurityWrite), | |
| 1080 | 1081 | ("get_security_settings", Scope::SecurityRead), | |
| 1081 | 1082 | ("get_workspace_security_settings", Scope::SecurityRead), | |
| 1082 | 1083 | ("get_security_overview", Scope::SecurityRead), |
| 6071 | 6071 | body: &[], | |
| 6072 | 6072 | }, | |
| 6073 | 6073 | Operation { | |
| 6074 | + | id: "rerun_dependency_review", | |
| 6075 | + | method: "POST", | |
| 6076 | + | path: "/repos/{owner}/{name}/pulls/{number}/dependency-review/rerun", | |
| 6077 | + | tag: "Supply chain", | |
| 6078 | + | summary: "Re-run dependency review", | |
| 6079 | + | scope: Some("security:write"), | |
| 6080 | + | params: &[ | |
| 6081 | + | Param { name: "owner", location: In::Path, kind: "string", required: true, values: &[] }, | |
| 6082 | + | Param { name: "name", location: In::Path, kind: "string", required: true, values: &[] }, | |
| 6083 | + | Param { name: "number", location: In::Path, kind: "integer", required: true, values: &[] }, | |
| 6084 | + | ], | |
| 6085 | + | body: &[], | |
| 6086 | + | }, | |
| 6087 | + | Operation { | |
| 6074 | 6088 | id: "update_vulnerability_alert", | |
| 6075 | 6089 | method: "PATCH", | |
| 6076 | 6090 | path: "/repos/{owner}/{name}/vulnerability-alerts/{id}", |
| 6 | 6 | use std::collections::{BTreeMap, BTreeSet}; | |
| 7 | 7 | ||
| 8 | 8 | use serde::{Deserialize, Serialize}; | |
| 9 | + | use sha2::{Digest, Sha256}; | |
| 9 | 10 | ||
| 10 | 11 | use crate::graph::Dependency; | |
| 11 | 12 | use crate::osv::Severity; | |
| ⋯ | |||
| 111 | 112 | pub denied_license: bool, | |
| 112 | 113 | } | |
| 113 | 114 | ||
| 115 | + | /// How the pull request comment starts: g1t finds its own comment again | |
| 116 | + | /// by it, to change it rather than add another. | |
| 117 | + | pub const HEADING: &str = "### Dependency review"; | |
| 118 | + | ||
| 119 | + | /// The comment on a pull request that changes no dependencies. | |
| 120 | + | pub fn unchanged_summary() -> String { | |
| 121 | + | format!("{HEADING}\n\nThis pull request changes no dependencies.\n") | |
| 122 | + | } | |
| 123 | + | ||
| 124 | + | /// A short, stable key for `text`: what a review was of, so that the same | |
| 125 | + | /// change is not reviewed twice. | |
| 126 | + | pub fn key(text: &str) -> String { | |
| 127 | + | Sha256::digest(text.as_bytes())[..16].iter().map(|byte| format!("{byte:02x}")).collect() | |
| 128 | + | } | |
| 129 | + | ||
| 114 | 130 | /// The whole review. | |
| 115 | 131 | #[derive(Clone, Debug, PartialEq, Eq)] | |
| 116 | 132 | pub struct Verdict { | |
| ⋯ | |||
| 180 | 196 | /// The pull request comment: what changed, per lockfile, and why the | |
| 181 | 197 | /// check failed if it did. Markdown. | |
| 182 | 198 | pub fn summary(&self, policy: &Policy) -> String { | |
| 183 | − | let mut text = String::from("### Dependency review\n\n"); | |
| 184 | 199 | if self.added.is_empty() && self.removed.is_empty() { | |
| 185 | − | text.push_str("This pull request changes no dependencies.\n"); | |
| 186 | − | return text; | |
| 200 | + | return unchanged_summary(); | |
| 187 | 201 | } | |
| 202 | + | let mut text = format!("{HEADING}\n\n"); | |
| 188 | 203 | text.push_str(if self.passed { "**Passed.** " } else { "**Failed.** " }); | |
| 189 | 204 | text.push_str(&self.headline()); | |
| 190 | 205 | text.push_str(".\n\n"); | |
| ⋯ | |||
| 296 | 311 | } | |
| 297 | 312 | ||
| 298 | 313 | #[test] | |
| 314 | + | fn every_summary_starts_with_the_heading_g1t_finds_it_by() { | |
| 315 | + | let policy = Policy { fail_on: Some(Severity::High), deny_licenses: Vec::new() }; | |
| 316 | + | let none = judge(&[], &BTreeMap::new(), &policy); | |
| 317 | + | assert_eq!(none.summary(&policy), unchanged_summary()); | |
| 318 | + | let some = judge(&diff(&[], &[dep("chalk", "5.0.0", None)]), &BTreeMap::new(), &policy); | |
| 319 | + | assert!(unchanged_summary().starts_with(HEADING) && some.summary(&policy).starts_with(&format!("{HEADING}\n\n**Passed.**"))); | |
| 320 | + | assert_eq!(key("a"), key("a")); | |
| 321 | + | assert_ne!(key("a"), key("b")); | |
| 322 | + | assert_eq!(key("a").len(), 32); | |
| 323 | + | } | |
| 324 | + | ||
| 325 | + | #[test] | |
| 299 | 326 | fn the_review_fails_at_the_configured_severity() { | |
| 300 | 327 | let changes = diff(&[dep("lodash", "4.17.21", None)], &[dep("lodash", "4.17.20", None)]); | |
| 301 | 328 | let findings = BTreeMap::from([("pkg:npm/lodash@4.17.20".to_owned(), vec![finding(Severity::High)])]); | |
| 445 | 445 | ["get_dependency_graph", "security:read"], | |
| 446 | 446 | ["get_sbom", "security:read"], | |
| 447 | 447 | ["compare_dependencies", "security:read"], | |
| 448 | + | ["rerun_dependency_review", "security:write"], | |
| 448 | 449 | ["get_security_settings", "security:read"], | |
| 449 | 450 | ["get_workspace_security_settings", "security:read"], | |
| 450 | 451 | ["get_security_overview", "security:read"], |
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Comments
Sign in to comment.