feat(vis)!: offline OSV scanner + unified security.policies#632
Merged
Conversation
…port Add a fully offline vulnerability scanner for `vis audit` backed by a local SQLite advisory DB plus a sibling `vis advisories` command suite for cache management (sync, prune, status). Scope (Phase 4 of the offline-scanner RFC): - Rust NAPI bindings for OSV ingest, SQLite query, and per-ecosystem range matchers (npm semver, PEP 440 via pep440_rs, Cargo SemVer 2.0, Maven 3, Go modules, RubyGems custom ordering). - JS lockfile readers for non-npm ecosystems: poetry.lock, uv.lock, Pipfile.lock, Cargo.lock, pom.xml, gradle.lockfile, go.sum, Gemfile.lock — regex/JSON only, no TOML/XML runtime dep. - `vis audit --offline` queries the local DB; supports `--ecosystem npm,pypi,crates.io,cargo,maven,go,rubygems` (canonical OSV names with cli-style aliases). - New output formats: SARIF 2.1.0, CSAF 2.0 csaf_vex, CycloneDX 1.7 + VEX, self-contained HTML report. - Direct-dep and transitive-override fix planners (`--apply`, `--apply-transitive`) with PM-aware writes. - Reachability filter (`--usage`) drops findings for packages not statically imported anywhere in the workspace. - vis-mcp surface: new `audit` and `advisory-status` tools wrapping the CLI for editor/agent integration. Cosign signature verification for the offline DB is deliberately deferred (tracked separately in issue #631) — OSV upstream doesn't sign today and sigstore-rs is heavy. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CSAF emitter is now spec-compliant (optional product_tree/vulnerabilities, valid flag.label enum). audit-html round-trips through happy-dom for real DOM-level assertions. Native range matchers carry an OSV corpus_tests module covering all six ecosystems. New __bench__/ folder holds the 2.8k-package advisoriesQuery perf gate (80ms target, 120ms slack) plus its bench() trend reporter, both sharing a STORED-zip fixture builder. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Renames the audit auto-fix flags to match what users expect from `npm audit fix` / `pnpm audit --fix`: - `--apply` → `--fix` (apply direct-dep upgrades, then rescan) - `--apply-transitive` → `--fix-transitive` - new `--show-fixes` flag takes over the old "print suggestions" behavior that `--fix` previously had Config namespace `security.audit.apply.transitive.enabled` is unchanged. Also defers Phase 4.22 (Cosign sigstore verification) to issue #631 in the RFC — the config schema stays wired so users can declare intent, but the dual-binary release plumbing isn't justified until OSV upstream ships signatures. Companion cleanup: drops 368 `// ── … ─────` separator-comment lines across 67 files in the vis package. Runtime box-drawing in TUI strings and audit/toolchain log output is preserved. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Hard rename of supply-chain knobs into a single security.policies.* block (vis is alpha, no aliases kept). Establishes the Socket-style 8-policy surface that the policy engine will plug into. BREAKING CHANGE: the following security.* keys were renamed: - security.minimumReleaseAge -> security.policies.first_seen.minutes - security.minimumReleaseAgeExclude -> security.policies.first_seen.exclude - security.trustPolicy -> security.policies.publisher_change.mode - security.trustPolicyExclude -> security.policies.publisher_change.exclude - security.trustPolicyIgnoreAfter -> security.policies.publisher_change.ignoreAfter - security.allowBuilds -> security.policies.install_scripts.allow - security.strictDepBuilds -> security.policies.install_scripts.strict - security.socket.minimumScore -> security.policies.score.minimum - security.socket.acceptedRisks -> security.acceptedRisks - security.audit.failOn -> security.policies.vulnerability.failOn - security.audit.usage -> security.policies.vulnerability.usage AcceptedRisk now carries optional policies[] and expiresAt fields so risks can be scoped per-policy and time-boxed. Native PM sync writers keep emitting pnpm-native field names since pnpm owns that schema. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
✅ Deploy Preview for visulima-com canceled.
|
Contributor
|
Thank you for following the naming conventions! 🙏 |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds native offline OSV advisories DB (ingest/query/range/schema/status) with N‑API bindings and JS exports, a new ChangesOffline advisories DB and audit CLI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tooling/vis/src/commands/hook/builtins/index.ts (1)
29-31: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd JSDoc comments to exported functions.
The exported functions
isBuiltinandgetBuiltinlack JSDoc documentation. As per coding guidelines, exported functions should include JSDoc comments with parameter types and return types to improve API clarity and developer experience.📝 Suggested JSDoc documentation
+/** + * Checks if a hook ID corresponds to a registered builtin hook. + * + * `@param` id - The hook identifier to check + * `@returns` True if the ID is a registered builtin, false otherwise + */ const isBuiltin = (id: string): boolean => Object.hasOwn(BUILTIN_REGISTRY, id); +/** + * Retrieves a builtin hook function by its ID. + * + * `@param` id - The builtin hook identifier + * `@returns` The builtin function if found, undefined otherwise + */ const getBuiltin = (id: string): BuiltinFunction | undefined => BUILTIN_REGISTRY[id];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/hook/builtins/index.ts` around lines 29 - 31, Add JSDoc comments to the exported functions isBuiltin and getBuiltin: above isBuiltin(id: string): boolean add a JSDoc block that documents the id parameter (string) and the boolean return value and describes that it checks membership in BUILTIN_REGISTRY; above getBuiltin(id: string): BuiltinFunction | undefined add a JSDoc block that documents the id parameter, the possible return types (BuiltinFunction or undefined) and describes that it retrieves an entry from BUILTIN_REGISTRY by id. Ensure both blocks are properly placed immediately above the function declarations and follow the project’s JSDoc style.
♻️ Duplicate comments (1)
packages/tooling/vis/src/commands/hook/dispatch.ts (1)
246-246:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winSame exit-code aggregation change affects stage-level failure handling.
This line has the same
|=→||=change as inrunShellCommand. The behavioral impact is identical: only the first non-zero exit code from any hook in the stage will be preserved, and subsequent hook failures won't updaterc.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/hook/dispatch.ts` at line 246, The stage-level exit-code aggregation incorrectly uses logical-or-assignment on the variable rc (the line "rc ||= code"), which preserves only the first non-zero code; revert this to bitwise-or aggregation so all failing hooks update rc (use rc |= code) and ensure rc is initialized to 0 in the surrounding function (the dispatch/hook-run loop in dispatch.ts) so exit codes from all hooks accumulate correctly.
🧹 Nitpick comments (25)
packages/tooling/vis/src/commands/hook/builtins/mixed-line-ending.ts (1)
22-29: ⚡ Quick winAdd full JSDoc tags for exported
runMixedLineEnding.The description exists, but exported function docs should include explicit
@paramand@returnsentries.As per coding guidelines
**/*.{ts,tsx,js,jsx}: Always include JSDoc comments for exported functions with parameter types and return types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/hook/builtins/mixed-line-ending.ts` around lines 22 - 29, Add full JSDoc tags to the exported runMixedLineEnding function: extend the existing comment block above runMixedLineEnding to include `@param` entries documenting files: ReadonlyArray<string>, args: ReadonlyArray<string>, and context: BuiltinContext (briefly state purpose of each), and an `@returns` tag describing the numeric exit code; ensure types in the tags match the function signature and keep the description concise.packages/tooling/vis/src/commands/hook/builtins/check-merge-conflict.ts (1)
30-36: ⚡ Quick winComplete JSDoc for exported hook entrypoint.
runCheckMergeConflictis exported but its JSDoc is missing typed@paramand@returnsfields.As per coding guidelines
**/*.{ts,tsx,js,jsx}: Always include JSDoc comments for exported functions with parameter types and return types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/hook/builtins/check-merge-conflict.ts` around lines 30 - 36, Add a complete JSDoc block for the exported function runCheckMergeConflict documenting each parameter and the return type: add `@param` tags for files: ReadonlyArray<string>, args: ReadonlyArray<string>, and context: BuiltinContext, and an `@returns` tag indicating it returns a number; ensure the description mirrors the existing top-line comment and the types match the function signature exactly so linters and readers can pick up typed documentation.packages/tooling/vis/src/commands/hook/prek.ts (1)
98-107: ⚡ Quick winAdd parameter/return tags to exported function JSDoc.
stageScriptBodyis exported, but the JSDoc still lacks explicit@paramand@returnsannotations.As per coding guidelines
**/*.{ts,tsx,js,jsx}: Always include JSDoc comments for exported functions with parameter types and return types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/hook/prek.ts` around lines 98 - 107, The exported function stageScriptBody lacks explicit JSDoc `@param` and `@returns` annotations; update its JSDoc to include an `@param` {string} stage description line and an `@returns` {string} description line so the comment documents the function's parameter and return type (referencing the exported stageScriptBody function).packages/tooling/vis/src/commands/docker/handler.ts (1)
8-8: ⚡ Quick winAdd JSDoc documentation for the exported function.
The
executefunction is exported but lacks JSDoc comments. As per coding guidelines, exported functions should include JSDoc with parameter types and return types, and functions should have comments describing their purpose.📝 Suggested JSDoc
+/** + * Executes Docker-related subcommands (scaffold or prune) for monorepo workspace management. + * + * `@param` toolbox - The command toolbox containing logger, options, workspace root, and vis config + * `@returns` A promise that resolves when the subcommand completes + * `@throws` {Error} If the subcommand is invalid or required options are missing + */ const execute = async ({ argument, logger, options, visConfig, workspaceRoot: wsRoot }: Toolbox<Console, DockerOptions>): Promise<void> => {As per coding guidelines: "Always include JSDoc comments for exported functions with parameter types and return types" and "Add a comment at the start of each function describing what it does".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/docker/handler.ts` at line 8, Add a JSDoc block above the exported async function execute to describe its purpose and document each parameter and the return type; mention Toolbox<Console, DockerOptions> and describe the named destructured parameters (argument, logger, options, visConfig, workspaceRoot/wsRoot) and that the function returns Promise<void>, and include any thrown errors or side effects in the description so the exported function has full JSDoc per project guidelines.packages/tooling/vis/src/commands/hook/builtins/trailing-whitespace.ts (1)
8-8: 💤 Low valueOptional: Align comment order with array order.
The comment on line 7 lists "SP, TAB, VT, FF, CR" but the array defines them as
[TAB, VT, FF, CR, SP]. While functionally equivalent (Set has no ordering), aligning the comment to match the array order would improve consistency.📝 Proposed comment fix
-// SP, TAB, VT, FF, CR +// TAB, VT, FF, CR, SP const WHITESPACE = new Set<number>([0x09, 0x0b, 0x0c, 0x0d, 0x20]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/hook/builtins/trailing-whitespace.ts` at line 8, The comment describing the WHITESPACE set is out of order relative to the array used to build it; update the inline comment that lists characters to match the array order used in the WHITESPACE constant (i.e., reflect [TAB, VT, FF, CR, SP] rather than "SP, TAB, VT, FF, CR") so the comment and the Set<number> initializer (WHITESPACE) are consistent.packages/tooling/vis/rfc/design-offline-vuln-scanner.md (1)
46-52: 💤 Low valueAdd language specifiers to fenced code blocks for better syntax highlighting.
Multiple code blocks are missing language identifiers, which would improve readability and enable proper syntax highlighting in documentation viewers.
🔧 Suggested fixes for code block language specifiers
Line 46-52 (shell/bash commands):
-``` +```bash vis advisories sync # Pull OSV npm dump → ~/.cache/vis/advisories/db.sqliteLine 58-72 (shell/bash commands):
-``` +```bash --offline Use the local DB; error if not syncedLine 266-303 (flow diagram/pseudocode):
-``` +```text ┌────────────────────────────────────────────┐Line 314-323 (flow diagram/pseudocode):
-``` +```text vis audit --offlineLine 369-386 (flow diagram/pseudocode):
-``` +```text ┌──────────────────────────────┐Also applies to: 58-72, 266-303, 314-323, 369-386
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/rfc/design-offline-vuln-scanner.md` around lines 46 - 52, Add appropriate fenced-code language tags for the undocumented blocks: change the shell command blocks containing "vis advisories sync", "--offline" and "vis audit --offline" to use ```bash (or ```sh) and mark the ASCII flow diagrams (blocks starting with "┌" and other box-drawing characters) as ```text. Update the code fences around the snippets containing the exact strings "vis advisories sync # Pull OSV npm dump", "--offline Use the local DB; error if not synced", the flow diagram beginning with "┌────────────────────────────────────────────┐", and the block with "vis audit --offline" so viewers get proper syntax highlighting.packages/tooling/vis/native/src/advisories/osv.rs (1)
207-283: ⚡ Quick winAdd test coverage for
last_affectedevent handling.The unit tests cover
introduced/fixedpairs and open ranges, but don't include scenarios withlast_affectedevents. Given the semantic nuance betweenlast_affected(inclusive) andfixed(exclusive), a test case would help document the intended behavior.🧪 Suggested test case
+ #[test] + fn range_with_last_affected_emits_pair() { + let r = Range { + kind: "SEMVER".into(), + events: vec![ + Event { + introduced: Some("1.0.0".into()), + fixed: None, + last_affected: None, + limit: None, + }, + Event { + introduced: None, + fixed: None, + last_affected: Some("1.5.9".into()), + limit: None, + }, + ], + }; + let pairs = r.to_pairs(); + assert_eq!(pairs.len(), 1); + assert_eq!(pairs[0].introduced, "1.0.0"); + assert_eq!(pairs[0].fixed.as_deref(), Some("1.5.9")); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/native/src/advisories/osv.rs` around lines 207 - 283, Add unit tests for Range::to_pairs that include Event::last_affected values: create a Range (using the Range and Event structs) with an introduced event followed by an event with last_affected Some("x"), and assert the produced pair has pair.last_affected set (pair.last_affected.as_deref() == Some("x")) and fixed is None; also add a test where last_affected appears alongside/around fixed to ensure to_pairs preserves fixed when present and correctly populates last_affected when provided. These tests should live alongside the existing tests referencing Range, Event, and to_pairs to document the inclusive semantics of last_affected.packages/tooling/vis/docs/commands/install.mdx (1)
15-15: 💤 Low valueConsider American English variant.
The documentation uses "afterwards" which is more common in British English. In American English, "afterward" (without the 's') is preferred. If the project follows American English conventions, consider this minor adjustment.
📝 Suggested edit
-2. **Lifecycle scripts blocked by default.** Mirrors pnpm v10: every PM (npm, pnpm, yarn, bun, aube) installs with `--ignore-scripts`. Packages listed in `security.policies.install_scripts.allow` (in `vis.config.ts`) get their `preinstall` / `install` / `postinstall` / `prepare` hooks executed afterwards by vis. Pass `--run-scripts` to restore the PM's native script behavior for one run. Deno does not run npm-style lifecycle scripts and is unaffected by these flags. +2. **Lifecycle scripts blocked by default.** Mirrors pnpm v10: every PM (npm, pnpm, yarn, bun, aube) installs with `--ignore-scripts`. Packages listed in `security.policies.install_scripts.allow` (in `vis.config.ts`) get their `preinstall` / `install` / `postinstall` / `prepare` hooks executed afterward by vis. Pass `--run-scripts` to restore the PM's native script behavior for one run. Deno does not run npm-style lifecycle scripts and is unaffected by these flags.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/docs/commands/install.mdx` at line 15, Change the wording "afterwards" to the American English variant "afterward" in the sentence that explains how packages listed in security.policies.install_scripts.allow get their preinstall / install / postinstall / prepare hooks executed (the sentence containing "get their `preinstall` / `install` / `postinstall` / `prepare` hooks executed afterwards by vis"). Update that single word so the documentation matches the project's American English convention.packages/tooling/vis-mcp/src/tools/advisory-status.ts (1)
8-21: ⚡ Quick winConsider using Zod schemas for runtime validation.
Similar to
audit.ts, theEcosystemSummaryandAdvisoryStatusJsoninterfaces only provide compile-time type checking. Since this tool parses external JSON output fromvis advisories status, using Zod schemas would provide runtime validation and better error messages if the JSON structure doesn't match expectations.♻️ Example refactor using Zod schemas
const ecosystemSummarySchema = z.object({ advisoryCount: z.number(), lastSyncIso: z.string(), manifestEtag: z.string().nullable(), name: z.string(), }); const advisoryStatusJsonSchema = z.object({ dbPath: z.string(), ecosystems: z.array(ecosystemSummarySchema), exists: z.boolean(), schemaVersion: z.number(), sizeBytes: z.number(), }); type AdvisoryStatusJson = z.infer<typeof advisoryStatusJsonSchema>;Then validate the payload:
const payload = advisoryStatusJsonSchema.parse(await execVisJson(...))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis-mcp/src/tools/advisory-status.ts` around lines 8 - 21, Replace the compile-time interfaces EcosystemSummary and AdvisoryStatusJson with runtime Zod schemas: create ecosystemSummarySchema and advisoryStatusJsonSchema (as shown in the review) and derive types with z.infer; then when parsing the external CLI JSON (where the code currently reads the vis advisories status output), validate the payload using advisoryStatusJsonSchema.parse(...) instead of trusting the raw object so malformed input throws a clear error. Ensure to import z from "zod" and update any references to use the inferred AdvisoryStatusJson type where needed.packages/tooling/vis-mcp/src/tools/audit.ts (1)
8-29: ⚡ Quick winConsider using Zod schemas for runtime validation.
The
VulnerabilityShape,ResultEntry, andAuditJsoninterfaces only provide compile-time type checking. Since this tool parses external JSON output fromvis audit, using Zod schemas would provide runtime validation and better error messages if the JSON structure doesn't match expectations.♻️ Example refactor using Zod schemas
const vulnerabilitySchema = z.object({ fixedVersions: z.array(z.string()).optional(), id: z.string(), severity: z.string(), summary: z.string().optional(), }); const resultEntrySchema = z.object({ acceptedRisk: z.unknown().optional(), name: z.string(), socketAlerts: z.array(z.unknown()).optional(), socketScore: z.number().nullable().optional(), version: z.string(), vulnerabilities: z.array(vulnerabilitySchema), }); const auditJsonSchema = z.object({ duplicates: z.array(z.object({ name: z.string(), versionCount: z.number(), versions: z.array(z.string()), })), packages: z.number(), results: z.array(resultEntrySchema), summary: z.object({ accepted: z.number(), duplicatePackages: z.number(), issues: z.number(), total: z.number(), }), }); type AuditJson = z.infer<typeof auditJsonSchema>;Then validate the payload:
const payload = auditJsonSchema.parse(await execVisJson(...))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis-mcp/src/tools/audit.ts` around lines 8 - 29, Replace the compile-time-only interfaces VulnerabilityShape, ResultEntry, and AuditJson with Zod schemas and use z.infer to derive types, then validate the parsed JSON before use; specifically, add vulnerabilitySchema, resultEntrySchema, and auditJsonSchema matching the existing fields (fixedVersions?: string[], id, severity, summary?, etc.), replace the AuditJson type with z.infer<typeof auditJsonSchema>, and call auditJsonSchema.parse(...) on the output you get from the vis audit JSON reader so malformed payloads throw a helpful runtime error.packages/tooling/vis/__bench__/audit-offline-fixture.ts (1)
25-89: 💤 Low valueConsider adding a comment explaining ZIP magic numbers.
The ZIP file format uses magic numbers like
0x04_03_4b_50which are standard but not immediately obvious. A brief comment explaining key magic numbers would improve readability.📝 Suggested comment
const buildStoredZip = (entries: ZipEntry[]): Buffer => { const localChunks: Buffer[] = []; const centralChunks: Buffer[] = []; let offset = 0; for (const entry of entries) { const nameBuf = Buffer.from(entry.name, "utf8"); const crc = crc32(entry.data); const size = entry.data.length; const local = Buffer.alloc(30); + // Local file header signature: PK\x03\x04 local.writeUInt32LE(0x04_03_4b_50, 0);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/__bench__/audit-offline-fixture.ts` around lines 25 - 89, Add brief explanatory comments in buildStoredZip to clarify the ZIP "magic" constants (e.g., 0x04034b50, 0x02014b50, 0x06054b50) and what each header block is (local file header, central directory header, end of central directory), and annotate key fields you write (crc, size, name length, offset) so readers can map the Buffer.writeUInt* calls to the ZIP spec; place comments near the local/central/eocd Buffer.alloc(...) blocks and next to the magic-number writeUInt32LE calls that currently use 0x04_03_4b_50, 0x02_01_4b_50, and 0x06_05_4b_50.packages/tooling/vis/src/commands/advisories/sync.ts (2)
11-12: ⚡ Quick winSimplify complex nested type or extract type alias.
The deeply-nested
NonNullablechain makes the return type difficult to read and maintain. Consider extracting a type alias.♻️ Refactor to use type alias
+type AdvisoriesConfig = NonNullable<NonNullable<NonNullable<VisConfig["security"]>["audit"]>["advisories"]>; + -const readAdvisoriesConfig = (visConfig: VisConfig | undefined): NonNullable<NonNullable<NonNullable<VisConfig["security"]>["audit"]>["advisories"]> => +const readAdvisoriesConfig = (visConfig: VisConfig | undefined): AdvisoriesConfig => visConfig?.security?.audit?.advisories ?? {};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/advisories/sync.ts` around lines 11 - 12, The return type for readAdvisoriesConfig is using a hard-to-read nested NonNullable chain; create a concise type alias (e.g., Advisories or VisSecurityAuditAdvisories) that resolves to the desired NonNullable<VisConfig["security"]>["audit"]>["advisories"] shape, then replace the long return type on readAdvisoriesConfig with that alias and keep the existing implementation (visConfig?.security?.audit?.advisories ?? {}), updating any imports/exports as needed so the codebase uses the new alias instead of repeating the nested NonNullable chain.
95-96: 💤 Low valueGuard array access to prevent potential undefined reference.
succeeded[0]?.result?.dbPathis accessed without verifying thatsucceededis non-empty, though the length check on line 95 makes this safe. However, the logic could be clearer.🛡️ More explicit guard
- if (succeeded.length > 0 && succeeded[0]?.result?.dbPath) { - pail.info(dim(`DB: ${succeeded[0].result.dbPath}`)); + const firstSuccess = succeeded[0]; + if (firstSuccess?.result?.dbPath) { + pail.info(dim(`DB: ${firstSuccess.result.dbPath}`)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/advisories/sync.ts` around lines 95 - 96, The code accesses succeeded[0]?.result?.dbPath even though the intent was to first verify succeeded is non-empty; make the guard explicit by capturing the first element and checking it before accessing its properties—e.g., const first = succeeded[0]; if (first && first.result && first.result.dbPath) { pail.info(dim(`DB: ${first.result.dbPath}`)); }—so use the succeeded variable and pail.info/dim symbols to locate and replace the current inline access with a clear null/undefined check.packages/tooling/vis/src/commands/audit/index.ts (2)
3-3: 💤 Low valueAdd JSDoc comment for the exported command.
The exported
auditcommand object lacks a JSDoc comment. As per coding guidelines, always include JSDoc comments for exported items.📝 Suggested JSDoc
+/** + * CLI command definition for `vis audit` — vulnerability and supply-chain security auditing. + * Supports offline OSV scanning, multiple ecosystems, fix workflows, and machine-readable outputs. + */ const audit: Command = {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/audit/index.ts` at line 3, Add a JSDoc comment above the exported Command object named audit describing its purpose, parameters (if any), return type, and any important behaviors or side effects; place the comment immediately above the declaration of the audit constant (symbol: audit) and follow existing project JSDoc style/formatting so tools and readers can pick up the documentation for the exported command.
140-159: 💤 Low valueAdd JSDoc comment for the exported type.
The exported
AuditOptionstype lacks a JSDoc comment. As per coding guidelines, always include JSDoc comments for exported items.📝 Suggested JSDoc
+/** + * Type-safe options for the `vis audit` command, derived from the CLI option declarations. + */ export type AuditOptions = CreateOptions<{🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/audit/index.ts` around lines 140 - 159, Add a JSDoc comment above the exported AuditOptions type declaration that briefly describes the purpose of the type (options for the audit command) and optionally lists/annotates the main properties or their intent (e.g., db, ecosystem, fix, format, severity, report, etc.) so it meets the project's exported-item documentation guideline; update the comment immediately above the export type AuditOptions to provide a one- or two-sentence summary and short descriptions for key fields.packages/tooling/vis/src/commands/audit/handler.ts (5)
909-989: 💤 Low valueAdd a comment describing the function's purpose.
As per coding guidelines, add a comment at the start of each function describing what it does.
📝 Suggested comment
+// Executes the --fix workflow: builds a direct-dependency upgrade plan, +// prompts for confirmation (unless --yes is set), applies the upgrades +// via the package manager, and returns an exit code or undefined to continue. const runApplyDirect = async (arguments_: RunApplyDirectArguments): Promise<number | undefined> => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/audit/handler.ts` around lines 909 - 989, Add a short JSDoc-style comment above the runApplyDirect function explaining its purpose: that it builds a DirectApplyPlan from RunApplyDirectArguments, displays and prompts the user for confirmation (respecting CI and --yes), groups planned fixes by workspace, runs the package manager add commands for each workspace using runAdd, and returns an exit code or undefined when there are no changes; reference the function name runApplyDirect and mention the key behaviors (building plan, prompting, grouping by workspace, invoking runAdd, and final success/exit handling) so future readers know what this function does at a glance.
72-82: 💤 Low valueAdd a comment describing the function's purpose.
As per coding guidelines, add a comment at the start of each function describing what it does.
📝 Suggested comment
+// Parses the --ecosystem CLI argument into a list of ecosystems and +// identifies any unsupported values for warning purposes. const parseEcosystems = (raw: string | undefined): { all: string[]; unsupported: string[] } => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/audit/handler.ts` around lines 72 - 82, Add a short JSDoc or inline comment above the parseEcosystems function explaining its purpose: that it parses a comma-separated raw ecosystems string (defaulting to "npm"), trims and filters entries, returns an object with the normalized list as `all` and any entries not in `SUPPORTED_ECOSYSTEMS` (case-insensitive) as `unsupported`; reference the parseEcosystems function name and SUPPORTED_ECOSYSTEMS constant so reviewers can quickly locate the implementation to verify behavior.
999-1087: 💤 Low valueAdd a comment describing the function's purpose.
As per coding guidelines, add a comment at the start of each function describing what it does.
📝 Suggested comment
+// Executes the --fix-transitive workflow: builds an override plan for +// vulnerable transitive dependencies, gates on config and CI checks, +// prompts for confirmation (unless --yes is set), and writes the overrides. const runApplyTransitive = async (arguments_: RunApplyTransitiveArguments): Promise<number | undefined> => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/audit/handler.ts` around lines 999 - 1087, The function runApplyTransitive is missing a leading comment describing its purpose; add a brief JSDoc or single-line comment immediately above the runApplyTransitive declaration that summarizes what the function does (e.g., applies transitive vulnerability fixes by building an overrides plan, showing the plan to the user, gating in CI and optional confirmation, then writing the overrides via applyOverridePlan), and mention key behaviors like CI gating, direct-dep filtering via buildDirectApplyPlan, and that it returns exit codes (0/1/undefined) to guide callers; place the comment directly above the const runApplyTransitive = async (...) declaration so it’s clear in editors and review tools.
872-895: 💤 Low valueAdd a comment describing the function's purpose.
As per coding guidelines, add a comment at the start of each function describing what it does.
📝 Suggested comment
+// Prompts the user for a yes/no answer in interactive TTY sessions. +// Falls back to the default when stdin is not a TTY (e.g., in CI). const promptYesNo = async (question: string, defaultYes: boolean): Promise<boolean> => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/audit/handler.ts` around lines 872 - 895, Add a short doc comment above the promptYesNo function explaining its purpose: that it prompts the user with a yes/no question using stdin/stdout (TTY only), returns the provided default when stdin is not a TTY or when the user submits an empty answer, and resolves to true for answers starting with "y" (case-insensitive); mention the parameters (question: string, defaultYes: boolean) and the Promise<boolean> return. Place this comment immediately above the promptYesNo declaration so future readers and linters can understand its behavior.
1090-1098: 💤 Low valueAdd JSDoc comment for the exported handler function.
The exported
executefunction lacks a JSDoc comment. As per coding guidelines, always include JSDoc comments for exported functions with parameter types and return types.📝 Suggested JSDoc
+/** + * Handler for the `vis audit` command. Executes the audit workflow including + * vulnerability scanning (online or offline), reachability filtering, fix planning, + * and output generation in multiple formats. + * + * `@param` toolbox - Command toolbox with logger, options, config, and workspace root + */ const execute = async ({ logger, options, visConfig, workspaceRoot: wsRoot }: Toolbox<Console, AuditOptions>): Promise<void> => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/audit/handler.ts` around lines 1090 - 1098, Add a JSDoc block above the exported execute function (`execute`) documenting the function purpose and each parameter: `logger`, `options` (type AuditOptions), `visConfig`, and `workspaceRoot` (from `Toolbox<Console, AuditOptions>`), and the return type `Promise<void>`; reference the types `Toolbox`, `AuditOptions`, and `CommandExecute` in the JSDoc `@param/`@returns tags and ensure the export line (`export default execute as CommandExecute<Toolbox>;`) remains unchanged.packages/tooling/vis/native/src/advisories_napi.rs (1)
167-167: 💤 Low valueDocument the u64 → u32 truncation behavior.
Lines 167 and 200 silently clamp u64 values to u32::MAX when converting for JS. While realistic advisory databases won't hit these limits, adding a comment would clarify the intended behavior and help future maintainers.
📝 Suggested comments
size_bytes: status.size_bytes.min(u32::MAX as u64) as u32, + // Clamp to u32::MAX (~4GB) for JS number compatibility; advisory DBs won't exceed this in practice.advisory_count: e.advisory_count.min(u32::MAX as u64) as u32, + // Clamp to u32::MAX for JS number compatibility; realistic advisory counts won't exceed this.Also applies to: 200-200
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/native/src/advisories_napi.rs` at line 167, Document the intentional truncation when converting u64 to u32 by adding a concise comment next to the conversion expressions (e.g., the size_bytes assignment that uses status.size_bytes.min(u32::MAX as u64) as u32 and the similar expression at the second occurrence) stating that values above u32::MAX are clamped to u32::MAX for JavaScript consumption and that this is deliberate because advisory DBs are not expected to exceed that range; reference the fields size_bytes and status.size_bytes (and the exact min(... as u64) as u32 pattern) so future maintainers understand the behavior.packages/tooling/vis/src/commands/advisories/index.ts (1)
102-119: 💤 Low valueConsider adding JSDoc comments for exported types.
The exported option types (
AdvisoriesSyncOptions,AdvisoriesStatusOptions,AdvisoriesPruneOptions) lack JSDoc comments. While the command definitions above provide context, adding brief JSDoc would improve discoverability. As per coding guidelines, exported items should include JSDoc comments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/advisories/index.ts` around lines 102 - 119, Add concise JSDoc comments above the exported option types AdvisoriesSyncOptions, AdvisoriesStatusOptions, and AdvisoriesPruneOptions describing their purpose and the meaning of each field (e.g., db, ecosystem, force, format, source) so they are discoverable in IDEs and documentation; place a brief one-line summary and short per-field descriptions in /** ... */ immediately above each type declaration, following the project's JSDoc style conventions.packages/tooling/vis/src/commands/hook/discover.ts (1)
10-10: ⚡ Quick winConsider multi-line formatting for union type readability.
The single-line union type with three object variants is harder to read and maintain compared to the original multi-line format. Union types with multiple complex variants typically benefit from line breaks.
♻️ Suggested multi-line formatting
-export type DiscoverMode = { kind: "all" } | { fromRef: string; kind: "range"; toRef: string } | { kind: "staged" }; +export type DiscoverMode = + | { kind: "all" } + | { kind: "staged" } + | { fromRef: string; kind: "range"; toRef: string };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/hook/discover.ts` at line 10, The union type exported as DiscoverMode is currently on one line and is hard to read; reformat the export type DiscoverMode so each variant is on its own line (one line for { kind: "all" }, one for { fromRef: string; kind: "range"; toRef: string }, and one for { kind: "staged" }) to improve readability and maintainability, keeping the same type names and shape but using multi-line formatting for the union members.packages/tooling/vis/src/commands/hook/run.ts (1)
62-62: 💤 Low valueConsider multi-line formatting for function signature readability.
The single-line function signature with four parameters is harder to scan. Multi-line parameter formatting improves readability for functions with multiple parameters, especially when parameter names and types are descriptive.
♻️ Suggested multi-line formatting
-const runHookStage = (root: string, hooksDirectory: string, options: RunOptions, logger: RunLogger): number => { +const runHookStage = ( + root: string, + hooksDirectory: string, + options: RunOptions, + logger: RunLogger, +): number => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/hook/run.ts` at line 62, The function signature for runHookStage(root: string, hooksDirectory: string, options: RunOptions, logger: RunLogger) is on a single line and is hard to scan; reformat the signature to multiple lines so each parameter is on its own line (or grouped logically) to improve readability — locate the runHookStage declaration and break the parameter list across lines (keeping the same types and order: root, hooksDirectory, options, logger) and ensure indentation and surrounding formatting match the file's style.packages/tooling/vis/src/commands/hook/dispatch.ts (1)
58-63: 💤 Low valueSimplify logger wrapper using arrow functions.
The refactor from arrow function bodies to explicit block bodies adds unnecessary verbosity without improving clarity. The original arrow-return style was more idiomatic and concise for simple forwarding functions.
♻️ Revert to concise arrow style
const builtinLoggerFor = (parent: DispatchLogger): BuiltinLogger => { return { - error: (message) => { parent.error(message); }, - info: (message) => { parent.info(message); }, + error: (message) => parent.error(message), + info: (message) => parent.info(message), }; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/hook/dispatch.ts` around lines 58 - 63, The wrapper function builtinLoggerFor unnecessarily uses block-bodied arrow functions; revert to concise implicit-return arrow syntax for the inner methods to remove verbosity. Update builtinLoggerFor to return { error: (m) => parent.error(m), info: (m) => parent.info(m) } (keeping types DispatchLogger and BuiltinLogger intact) so the behavior is identical but the implementation is more idiomatic and concise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b7289492-c95e-44ba-8b36-20f496225098
⛔ Files ignored due to path filters (48)
packages/tooling/vis-mcp/__tests__/__fixtures__/fake-vis.mjsis excluded by!**/__tests__/**,!**/__fixtures__/**and included bypackages/**packages/tooling/vis-mcp/__tests__/server.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis-mcp/__tests__/tools.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__bench__/audit-offline.test.tsis excluded by!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/commands/doctor/supply-chain.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/commands/hook/hook-commands.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/commands/hook/hook-prek.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/commands/hook/hook.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/commands/ignore/ignore.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/commands/init/init.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/commands/migrate/migrate.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/commands/pm/commands-pm.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/config/config.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/fixtures/schemas/cryptography-defs.schema.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/fixtures/schemas/csaf-2.0.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/fixtures/schemas/cvss-v2.0.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/fixtures/schemas/cvss-v3.0.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/fixtures/schemas/cvss-v3.1.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/fixtures/schemas/cyclonedx-1.7.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/fixtures/schemas/jsf-0.82.schema.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/fixtures/schemas/load.tsis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/fixtures/schemas/sarif-2.1.0.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/fixtures/schemas/spdx.schema.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/native/native-binding.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/pm/optimize.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/pm/overrides.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/report/audit-html.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/report/csaf.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/report/cyclonedx-vex.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/report/sarif.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/sbom/schemas/bom-1.7.schema.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/sbom/schemas/cryptography-defs.schema.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/sbom/schemas/jsf-0.82.schema.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/sbom/schemas/spdx.schema.jsonis excluded by!**/__tests__/**and included bypackages/**packages/tooling/vis/__tests__/schemas/schemas.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/advisories.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/apply-direct.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/dependency-scan.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/multi-eco-lockfiles.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/reachability.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/script-security.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/security.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/transitive-fix.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/typosquats.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/tui/status-utils.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/util/aube-resolver.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/native/Cargo.lockis excluded by!**/*.lockand included bypackages/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by none
📒 Files selected for processing (138)
packages/tooling/vis-mcp/src/server.tspackages/tooling/vis-mcp/src/tools/advisory-status.tspackages/tooling/vis-mcp/src/tools/audit.tspackages/tooling/vis/LICENSE.mdpackages/tooling/vis/__bench__/audit-offline-fixture.tspackages/tooling/vis/__bench__/audit-offline.bench.tspackages/tooling/vis/competitive-analysis.mdpackages/tooling/vis/docs/commands/add.mdxpackages/tooling/vis/docs/commands/advisories.mdxpackages/tooling/vis/docs/commands/approve-builds.mdxpackages/tooling/vis/docs/commands/audit.mdxpackages/tooling/vis/docs/commands/install.mdxpackages/tooling/vis/docs/commands/update.mdxpackages/tooling/vis/docs/configuration.mdxpackages/tooling/vis/docs/guides/security-audit.mdxpackages/tooling/vis/docs/guides/why-vis.mdxpackages/tooling/vis/docs/index.mdxpackages/tooling/vis/docs/meta.jsonpackages/tooling/vis/eslint.config.jspackages/tooling/vis/index.jspackages/tooling/vis/native/Cargo.tomlpackages/tooling/vis/native/src/advisories/ingest.rspackages/tooling/vis/native/src/advisories/mod.rspackages/tooling/vis/native/src/advisories/osv.rspackages/tooling/vis/native/src/advisories/query.rspackages/tooling/vis/native/src/advisories/range/cargo.rspackages/tooling/vis/native/src/advisories/range/corpus_tests.rspackages/tooling/vis/native/src/advisories/range/go.rspackages/tooling/vis/native/src/advisories/range/maven.rspackages/tooling/vis/native/src/advisories/range/mod.rspackages/tooling/vis/native/src/advisories/range/npm.rspackages/tooling/vis/native/src/advisories/range/pep440.rspackages/tooling/vis/native/src/advisories/range/rubygems.rspackages/tooling/vis/native/src/advisories/schema.rspackages/tooling/vis/native/src/advisories/status.rspackages/tooling/vis/native/src/advisories_napi.rspackages/tooling/vis/native/src/lib.rspackages/tooling/vis/native/src/pm_resolve.rspackages/tooling/vis/package.jsonpackages/tooling/vis/packem.config.tspackages/tooling/vis/priority-roadmap.mdpackages/tooling/vis/rfc/design-offline-vuln-scanner.mdpackages/tooling/vis/schemas/vis-config.schema.jsonpackages/tooling/vis/scripts/sync-blocklist.tspackages/tooling/vis/src/bin.tspackages/tooling/vis/src/commands/add/handler.tspackages/tooling/vis/src/commands/add/index.tspackages/tooling/vis/src/commands/advisories/index.tspackages/tooling/vis/src/commands/advisories/prune.tspackages/tooling/vis/src/commands/advisories/status.tspackages/tooling/vis/src/commands/advisories/sync.tspackages/tooling/vis/src/commands/analyze/handler.tspackages/tooling/vis/src/commands/approve-builds/handler.tspackages/tooling/vis/src/commands/audit/handler.tspackages/tooling/vis/src/commands/audit/index.tspackages/tooling/vis/src/commands/check/handler.tspackages/tooling/vis/src/commands/create/discovery.tspackages/tooling/vis/src/commands/create/handler.tspackages/tooling/vis/src/commands/create/prompts.tspackages/tooling/vis/src/commands/create/templates/builtin.tspackages/tooling/vis/src/commands/create/templates/generator.tspackages/tooling/vis/src/commands/create/templates/monorepo.tspackages/tooling/vis/src/commands/create/templates/remote.tspackages/tooling/vis/src/commands/create/utils.tspackages/tooling/vis/src/commands/docker/handler.tspackages/tooling/vis/src/commands/doctor/handler.tspackages/tooling/vis/src/commands/doctor/supply-chain.tspackages/tooling/vis/src/commands/generate/handler.tspackages/tooling/vis/src/commands/graph/handler.tspackages/tooling/vis/src/commands/hook/builtins/check-json.tspackages/tooling/vis/src/commands/hook/builtins/check-merge-conflict.tspackages/tooling/vis/src/commands/hook/builtins/index.tspackages/tooling/vis/src/commands/hook/builtins/mixed-line-ending.tspackages/tooling/vis/src/commands/hook/builtins/trailing-whitespace.tspackages/tooling/vis/src/commands/hook/config.tspackages/tooling/vis/src/commands/hook/discover.tspackages/tooling/vis/src/commands/hook/dispatch.tspackages/tooling/vis/src/commands/hook/filter.tspackages/tooling/vis/src/commands/hook/prek.tspackages/tooling/vis/src/commands/hook/run.tspackages/tooling/vis/src/commands/hook/validate.tspackages/tooling/vis/src/commands/ignore-helpers.tspackages/tooling/vis/src/commands/init/handler.tspackages/tooling/vis/src/commands/install/handler.tspackages/tooling/vis/src/commands/install/index.tspackages/tooling/vis/src/commands/migrate/lint-staged.tspackages/tooling/vis/src/commands/migrate/moon.tspackages/tooling/vis/src/commands/migrate/nx.tspackages/tooling/vis/src/commands/optimize/handler.tspackages/tooling/vis/src/commands/update/handler.tspackages/tooling/vis/src/config/audit-config.tspackages/tooling/vis/src/config/config.tspackages/tooling/vis/src/config/types.tspackages/tooling/vis/src/generate/moon-adapter/filters.tspackages/tooling/vis/src/generate/moon-adapter/index.tspackages/tooling/vis/src/generate/moon-adapter/tera-subset.tspackages/tooling/vis/src/generate/runner.tspackages/tooling/vis/src/io/symbols.tspackages/tooling/vis/src/plugins/security-enforcement.tspackages/tooling/vis/src/report/audit-html.tspackages/tooling/vis/src/report/csaf.tspackages/tooling/vis/src/report/cyclonedx-vex.tspackages/tooling/vis/src/report/sarif.tspackages/tooling/vis/src/runtime/toolchain.tspackages/tooling/vis/src/sbom/types.tspackages/tooling/vis/src/security/advisories.tspackages/tooling/vis/src/security/apply-direct.tspackages/tooling/vis/src/security/dependency-scan.tspackages/tooling/vis/src/security/multi-eco-lockfiles.tspackages/tooling/vis/src/security/reachability.tspackages/tooling/vis/src/security/security.tspackages/tooling/vis/src/security/socket-security.tspackages/tooling/vis/src/security/transitive-fix.tspackages/tooling/vis/src/security/typosquats.tspackages/tooling/vis/src/task/selectors.tspackages/tooling/vis/src/tui/components/devcontainer/devcontainer-store.tspackages/tooling/vis/src/tui/components/devcontainer/types.tspackages/tooling/vis/src/tui/components/devcontainer/vis-devcontainer-app.tsxpackages/tooling/vis/src/tui/components/graph/graph-store.tspackages/tooling/vis/src/tui/components/graph/project-detail-panel.tsxpackages/tooling/vis/src/tui/components/graph/project-list-panel.tsxpackages/tooling/vis/src/tui/components/graph/vis-graph-app.tsxpackages/tooling/vis/src/tui/components/migrate/migrate-store.tspackages/tooling/vis/src/tui/components/output-panel.tsxpackages/tooling/vis/src/tui/components/service-dock/service-dock.tsxpackages/tooling/vis/src/tui/components/task-list-panel.tsxpackages/tooling/vis/src/tui/components/task-store.tspackages/tooling/vis/src/tui/components/update/package-detail-panel.tsxpackages/tooling/vis/src/tui/components/update/package-list-panel.tsxpackages/tooling/vis/src/tui/components/update/update-store.tspackages/tooling/vis/src/tui/components/update/vis-update-app.tsxpackages/tooling/vis/src/tui/components/vis-task-runner-app.tsxpackages/tooling/vis/src/util/catalog.tspackages/tooling/vis/src/util/docker-lockfile/yarn.tspackages/tooling/vis/src/util/docker.tspackages/tooling/vis/src/util/identify.tspackages/tooling/vis/src/util/tips.tspackages/tooling/vis/src/util/utils.ts
💤 Files with no reviewable changes (17)
- packages/tooling/vis/src/commands/create/templates/monorepo.ts
- packages/tooling/vis/src/commands/create/prompts.ts
- packages/tooling/vis/src/commands/graph/handler.ts
- packages/tooling/vis/packem.config.ts
- packages/tooling/vis/src/commands/migrate/moon.ts
- packages/tooling/vis/src/commands/create/utils.ts
- packages/tooling/vis/src/commands/create/templates/builtin.ts
- packages/tooling/vis/src/commands/create/discovery.ts
- packages/tooling/vis/src/commands/migrate/lint-staged.ts
- packages/tooling/vis/native/src/pm_resolve.rs
- packages/tooling/vis/scripts/sync-blocklist.ts
- packages/tooling/vis/src/commands/create/templates/remote.ts
- packages/tooling/vis/src/commands/create/templates/generator.ts
- packages/tooling/vis/src/commands/ignore-helpers.ts
- packages/tooling/vis/src/commands/create/handler.ts
- packages/tooling/vis/src/commands/migrate/nx.ts
- packages/tooling/vis/src/commands/generate/handler.ts
Comment on lines
+23
to
+55
| export const registerAdvisoryStatus = ({ server }: ToolDeps, context: ToolContext): void => { | ||
| server.registerTool( | ||
| "advisory_status", | ||
| { | ||
| annotations: { readOnlyHint: true }, | ||
| description: | ||
| "Report the local OSV advisory DB freshness via `vis advisories status --format json`: " | ||
| + "DB path, schema version, per-ecosystem advisory counts, last sync timestamp, and ETag. " | ||
| + "Use this to detect whether the offline cache is stale before running an `audit` in offline mode.", | ||
| inputSchema: { | ||
| db: z | ||
| .string() | ||
| .optional() | ||
| .describe("Override the cache DB path (default: <cache>/vis/advisories/db.sqlite)."), | ||
| }, | ||
| }, | ||
| async (input: { db?: string }) => { | ||
| try { | ||
| const args = ["advisories", "status", "--format", "json"]; | ||
|
|
||
| if (input.db) { | ||
| args.push("--db", input.db); | ||
| } | ||
|
|
||
| const payload = await execVisJson<AdvisoryStatusJson>(context.visBin, args, { cwd: context.workspaceRoot }); | ||
|
|
||
| return okResponse(payload); | ||
| } catch (error) { | ||
| return errorResponse(error); | ||
| } | ||
| }, | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc comment for the exported function.
The registerAdvisoryStatus function is exported but lacks a JSDoc comment describing its purpose, parameters, and behavior. As per coding guidelines, exported functions should include JSDoc with parameter types and return types.
📚 Suggested JSDoc
+/**
+ * Registers the 'advisory_status' MCP tool that reports local OSV advisory DB status.
+ * The tool shows DB freshness, schema version, per-ecosystem advisory counts, last sync
+ * timestamps, and ETags to help determine if the offline cache needs updating.
+ *
+ * `@param` deps - Tool dependencies containing the MCP server instance
+ * `@param` context - Tool context with workspace root and vis binary path
+ */
export const registerAdvisoryStatus = ({ server }: ToolDeps, context: ToolContext): void => {As per coding guidelines, exported functions in TypeScript files should include JSDoc comments with parameter types and return types.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis-mcp/src/tools/advisory-status.ts` around lines 23 - 55,
Add a JSDoc block above the exported function registerAdvisoryStatus that
briefly describes its purpose (registers the "advisory_status" tool to report
local OSV advisory DB freshness), documents the parameters (server: ToolDeps and
context: ToolContext) including expected shapes/types, and states the return
type (void) and error behavior (register callback may return okResponse or
errorResponse). Ensure the JSDoc references the function name
registerAdvisoryStatus and the input parameter shape (input: { db?: string }) so
callers/readers understand overrides and side effects.
Comment on lines
+33
to
+112
| export const registerAudit = ({ server }: ToolDeps, context: ToolContext): void => { | ||
| server.registerTool( | ||
| "audit", | ||
| { | ||
| annotations: { readOnlyHint: true }, | ||
| description: | ||
| "Audit installed packages for known vulnerabilities and supply-chain risk via `vis audit --format json`. " | ||
| + "Defaults to the local offline OSV cache when one exists, otherwise falls back to the live OSV API. " | ||
| + "Returns the parsed audit payload (results, duplicates, summary).", | ||
| inputSchema: { | ||
| ecosystem: z | ||
| .string() | ||
| .optional() | ||
| .describe("Comma-separated OSV ecosystems to scan. Defaults to 'npm' (the only matcher wired today)."), | ||
| offline: z | ||
| .boolean() | ||
| .optional() | ||
| .describe("Force the local OSV cache. Errors if the cache is missing — run `vis advisories sync` first."), | ||
| prodOnly: z | ||
| .boolean() | ||
| .optional() | ||
| .describe("Skip devDependencies — scan the production graph only."), | ||
| severity: z | ||
| .enum(SEVERITY_VALUES) | ||
| .optional() | ||
| .describe("Minimum severity to report. Defaults to 'low'."), | ||
| showAccepted: z | ||
| .boolean() | ||
| .optional() | ||
| .describe("Include findings already on the workspace's accepted-risk list."), | ||
| usage: z | ||
| .boolean() | ||
| .optional() | ||
| .describe("Apply the reachability filter — only report vulnerabilities in statically-imported packages."), | ||
| }, | ||
| }, | ||
| async (input: { | ||
| ecosystem?: string; | ||
| offline?: boolean; | ||
| prodOnly?: boolean; | ||
| severity?: typeof SEVERITY_VALUES[number]; | ||
| showAccepted?: boolean; | ||
| usage?: boolean; | ||
| }) => { | ||
| try { | ||
| const args = ["audit", "--format", "json"]; | ||
|
|
||
| if (input.severity) { | ||
| args.push("--severity", input.severity); | ||
| } | ||
|
|
||
| if (input.offline) { | ||
| args.push("--offline"); | ||
| } | ||
|
|
||
| if (input.prodOnly) { | ||
| args.push("--prod-only"); | ||
| } | ||
|
|
||
| if (input.usage) { | ||
| args.push("--usage"); | ||
| } | ||
|
|
||
| if (input.ecosystem) { | ||
| args.push("--ecosystem", input.ecosystem); | ||
| } | ||
|
|
||
| if (input.showAccepted) { | ||
| args.push("--show-accepted"); | ||
| } | ||
|
|
||
| const payload = await execVisJson<AuditJson>(context.visBin, args, { cwd: context.workspaceRoot }); | ||
|
|
||
| return okResponse(payload); | ||
| } catch (error) { | ||
| return errorResponse(error); | ||
| } | ||
| }, | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc comment for the exported function.
The registerAudit function is exported but lacks a JSDoc comment describing its purpose, parameters, and behavior. As per coding guidelines, exported functions should include JSDoc with parameter types and return types.
📚 Suggested JSDoc
+/**
+ * Registers the 'audit' MCP tool that runs vis audit for vulnerability scanning.
+ * The tool accepts optional filters for ecosystem, severity, offline mode, production-only,
+ * reachability analysis, and accepted risk display.
+ *
+ * `@param` deps - Tool dependencies containing the MCP server instance
+ * `@param` context - Tool context with workspace root and vis binary path
+ */
export const registerAudit = ({ server }: ToolDeps, context: ToolContext): void => {As per coding guidelines, exported functions in TypeScript files should include JSDoc comments with parameter types and return types.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const registerAudit = ({ server }: ToolDeps, context: ToolContext): void => { | |
| server.registerTool( | |
| "audit", | |
| { | |
| annotations: { readOnlyHint: true }, | |
| description: | |
| "Audit installed packages for known vulnerabilities and supply-chain risk via `vis audit --format json`. " | |
| + "Defaults to the local offline OSV cache when one exists, otherwise falls back to the live OSV API. " | |
| + "Returns the parsed audit payload (results, duplicates, summary).", | |
| inputSchema: { | |
| ecosystem: z | |
| .string() | |
| .optional() | |
| .describe("Comma-separated OSV ecosystems to scan. Defaults to 'npm' (the only matcher wired today)."), | |
| offline: z | |
| .boolean() | |
| .optional() | |
| .describe("Force the local OSV cache. Errors if the cache is missing — run `vis advisories sync` first."), | |
| prodOnly: z | |
| .boolean() | |
| .optional() | |
| .describe("Skip devDependencies — scan the production graph only."), | |
| severity: z | |
| .enum(SEVERITY_VALUES) | |
| .optional() | |
| .describe("Minimum severity to report. Defaults to 'low'."), | |
| showAccepted: z | |
| .boolean() | |
| .optional() | |
| .describe("Include findings already on the workspace's accepted-risk list."), | |
| usage: z | |
| .boolean() | |
| .optional() | |
| .describe("Apply the reachability filter — only report vulnerabilities in statically-imported packages."), | |
| }, | |
| }, | |
| async (input: { | |
| ecosystem?: string; | |
| offline?: boolean; | |
| prodOnly?: boolean; | |
| severity?: typeof SEVERITY_VALUES[number]; | |
| showAccepted?: boolean; | |
| usage?: boolean; | |
| }) => { | |
| try { | |
| const args = ["audit", "--format", "json"]; | |
| if (input.severity) { | |
| args.push("--severity", input.severity); | |
| } | |
| if (input.offline) { | |
| args.push("--offline"); | |
| } | |
| if (input.prodOnly) { | |
| args.push("--prod-only"); | |
| } | |
| if (input.usage) { | |
| args.push("--usage"); | |
| } | |
| if (input.ecosystem) { | |
| args.push("--ecosystem", input.ecosystem); | |
| } | |
| if (input.showAccepted) { | |
| args.push("--show-accepted"); | |
| } | |
| const payload = await execVisJson<AuditJson>(context.visBin, args, { cwd: context.workspaceRoot }); | |
| return okResponse(payload); | |
| } catch (error) { | |
| return errorResponse(error); | |
| } | |
| }, | |
| ); | |
| }; | |
| /** | |
| * Registers the 'audit' MCP tool that runs vis audit for vulnerability scanning. | |
| * The tool accepts optional filters for ecosystem, severity, offline mode, production-only, | |
| * reachability analysis, and accepted risk display. | |
| * | |
| * `@param` deps - Tool dependencies containing the MCP server instance | |
| * `@param` context - Tool context with workspace root and vis binary path | |
| */ | |
| export const registerAudit = ({ server }: ToolDeps, context: ToolContext): void => { | |
| server.registerTool( | |
| "audit", | |
| { | |
| annotations: { readOnlyHint: true }, | |
| description: | |
| "Audit installed packages for known vulnerabilities and supply-chain risk via `vis audit --format json`. " | |
| "Defaults to the local offline OSV cache when one exists, otherwise falls back to the live OSV API. " | |
| "Returns the parsed audit payload (results, duplicates, summary).", | |
| inputSchema: { | |
| ecosystem: z | |
| .string() | |
| .optional() | |
| .describe("Comma-separated OSV ecosystems to scan. Defaults to 'npm' (the only matcher wired today)."), | |
| offline: z | |
| .boolean() | |
| .optional() | |
| .describe("Force the local OSV cache. Errors if the cache is missing — run `vis advisories sync` first."), | |
| prodOnly: z | |
| .boolean() | |
| .optional() | |
| .describe("Skip devDependencies — scan the production graph only."), | |
| severity: z | |
| .enum(SEVERITY_VALUES) | |
| .optional() | |
| .describe("Minimum severity to report. Defaults to 'low'."), | |
| showAccepted: z | |
| .boolean() | |
| .optional() | |
| .describe("Include findings already on the workspace's accepted-risk list."), | |
| usage: z | |
| .boolean() | |
| .optional() | |
| .describe("Apply the reachability filter — only report vulnerabilities in statically-imported packages."), | |
| }, | |
| }, | |
| async (input: { | |
| ecosystem?: string; | |
| offline?: boolean; | |
| prodOnly?: boolean; | |
| severity?: typeof SEVERITY_VALUES[number]; | |
| showAccepted?: boolean; | |
| usage?: boolean; | |
| }) => { | |
| try { | |
| const args = ["audit", "--format", "json"]; | |
| if (input.severity) { | |
| args.push("--severity", input.severity); | |
| } | |
| if (input.offline) { | |
| args.push("--offline"); | |
| } | |
| if (input.prodOnly) { | |
| args.push("--prod-only"); | |
| } | |
| if (input.usage) { | |
| args.push("--usage"); | |
| } | |
| if (input.ecosystem) { | |
| args.push("--ecosystem", input.ecosystem); | |
| } | |
| if (input.showAccepted) { | |
| args.push("--show-accepted"); | |
| } | |
| const payload = await execVisJson<AuditJson>(context.visBin, args, { cwd: context.workspaceRoot }); | |
| return okResponse(payload); | |
| } catch (error) { | |
| return errorResponse(error); | |
| } | |
| }, | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis-mcp/src/tools/audit.ts` around lines 33 - 112, Add a
JSDoc block for the exported function registerAudit describing its purpose
(registers the "audit" tool that runs `vis audit --format json`), its parameters
(server: ToolDeps, context: ToolContext) and the structure of the input object
(ecosystem, offline, prodOnly, severity, showAccepted, usage) and the return
type (tool handler returns a Promise of okResponse or errorResponse wrapping
AuditJson). Reference the used symbols in the comment (registerAudit, ToolDeps,
ToolContext, AuditJson, execVisJson, okResponse, errorResponse) and briefly note
behavior on offline vs live OSV, severity filtering, and that it returns parsed
audit payload or an error; keep the JSDoc concise with `@param` and `@returns` tags.
Comment on lines
+112
to
+116
| export interface AuditOfflineFixture { | ||
| dbPath: string; | ||
| queries: AdvisoryQuery[]; | ||
| cleanup: () => void; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc comments for exported interface and function.
The exported AuditOfflineFixture interface and createAuditOfflineFixture function lack JSDoc documentation. As per coding guidelines, exported items should include JSDoc comments.
📝 Suggested JSDoc documentation
+/**
+ * Fixture for offline audit performance testing, containing a synthetic
+ * advisory database and matching query batch.
+ */
export interface AuditOfflineFixture {
dbPath: string;
queries: AdvisoryQuery[];
cleanup: () => void;
}
+/**
+ * Creates an offline audit fixture with N synthetic advisory entries.
+ * Generates a ZIP archive, ingests it into a temporary SQLite database,
+ * and returns the DB path with matching queries for benchmarking.
+ *
+ * `@param` pkgCount - Number of synthetic advisory packages to generate
+ * `@returns` Promise resolving to fixture with DB path, queries, and cleanup function
+ */
export const createAuditOfflineFixture = async (pkgCount: number): Promise<AuditOfflineFixture> => {As per coding guidelines, TypeScript files should include JSDoc comments for exported functions with parameter types and return types.
Also applies to: 118-143
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis/__bench__/audit-offline-fixture.ts` around lines 112 -
116, Add JSDoc comments for the exported interface AuditOfflineFixture and the
exported function createAuditOfflineFixture: document the purpose of
AuditOfflineFixture and each property (dbPath: string, queries: AdvisoryQuery[],
cleanup: () => void), and for createAuditOfflineFixture add a JSDoc block
describing the function, its parameters (their types) and the return type
(Promise<AuditOfflineFixture> or AuditOfflineFixture as appropriate) so both
exported symbols have clear descriptions and parameter/return annotations.
| } | ||
|
|
||
| ingested += 1; | ||
| if ingested % PROGRESS_EVERY == 0 { |
Contributor
There was a problem hiding this comment.
Fix clippy lint: use .is_multiple_of() instead of manual modulo check.
The pipeline flagged this line with clippy::manual_is_multiple_of. Replace the manual % check with the more idiomatic .is_multiple_of() method.
🔧 Proposed fix
- if ingested % PROGRESS_EVERY == 0 {
+ if ingested.is_multiple_of(PROGRESS_EVERY) {
progress.emit(ingested, total);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ingested % PROGRESS_EVERY == 0 { | |
| if ingested.is_multiple_of(PROGRESS_EVERY) { | |
| progress.emit(ingested, total); | |
| } |
🧰 Tools
🪛 GitHub Actions: Cargo Test / 1_Test vis.txt
[error] 221-221: Clippy error (manual-is-multiple-of): manual implementation of .is_multiple_of(). At if ingested % PROGRESS_EVERY == 0 {}. Replace with ingested.is_multiple_of(PROGRESS_EVERY) (flagged by -D warnings, clippy::manual_is_multiple_of).
🪛 GitHub Actions: Cargo Test / Test vis
[error] 221-221: cargo clippy -- -D warnings failed: clippy::manual-is-multiple-of. Manual implementation of .is_multiple_of(); replace ingested % PROGRESS_EVERY == 0 with ingested.is_multiple_of(PROGRESS_EVERY).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis/native/src/advisories/ingest.rs` at line 221, Replace
the manual modulo check "if ingested % PROGRESS_EVERY == 0" with the idiomatic
integer method to satisfy clippy: call ingested.is_multiple_of(PROGRESS_EVERY)
in the same conditional. Update the conditional using the ingested variable and
the PROGRESS_EVERY constant (in the same scope in ingest.rs) so the logic
remains identical but uses .is_multiple_of(...) instead of the % operator.
Comment on lines
+107
to
+113
| } else if let Some(last) = &event.last_affected { | ||
| if let Some(prev) = current_introduced.take() { | ||
| out.push(RangePair { | ||
| introduced: prev, | ||
| fixed: Some(last.clone()), | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find usages of RangePair in range matching logic
rg -nP --type=rust -C5 'RangePair\s*\{|\.fixed|to_pairs\(\)' --glob '**/range/**/*.rs' --glob '**/advisories/*.rs'
# Look for test files that might clarify the semantics
fd -e rs -x grep -l 'last_affected' {} \; | head -20Repository: visulima/visulima
Length of output: 15917
🏁 Script executed:
# Check if corpus_tests contain any last_affected cases in test data
grep -r "last_affected" packages/tooling/vis/native/src/advisories/range/corpus_tests.rs
# Find the matcher implementations to understand how they interpret fixed
fd -t f -e rs packages/tooling/vis/native/src/advisories/range/ | grep -E 'matcher|match' | head -10
# Look at the matcher trait/interface definition
rg -A10 'fn matches\(' packages/tooling/vis/native/src/advisories/range/ --type=rustRepository: visulima/visulima
Length of output: 7945
🏁 Script executed:
# Get full npm matcher implementation to see fixed comparison logic
sed -n '/fn matches/,/^ }/p' packages/tooling/vis/native/src/advisories/range/npm.rs | head -50
# Check rubygems too for comparison
sed -n '/fn matches/,/^ }/p' packages/tooling/vis/native/src/advisories/range/rubygems.rs | head -50
# Search for last_affected in corpus test file to see if it's tested
head -100 packages/tooling/vis/native/src/advisories/range/corpus_tests.rs
# Check OSV schema/documentation comments about last_affected
rg -B3 -A3 'last_affected' packages/tooling/vis/native/src/advisories/osv.rs | head -50Repository: visulima/visulima
Length of output: 6605
Fix last_affected boundary semantics in RangePair.fixed assignment.
The matchers treat fixed as an exclusive upper bound: installed >= fixed means unaffected. However, OSV's last_affected represents an inclusive upper bound—the last version that is affected. Assigning last_affected directly to fixed (line 111) creates an off-by-one error: version 1.2.3 would be treated as unaffected when it should be affected.
Either increment last_affected before assigning it to fixed, or modify the matching logic to handle last-affected ranges inclusively. The corpus tests appear to lack coverage for last_affected scenarios; add a test case with real advisories using last_affected events to validate the fix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis/native/src/advisories/osv.rs` around lines 107 - 113,
The code assigns OSV event.last_affected directly into RangePair.fixed, but
RangePair.fixed is treated as an exclusive upper bound (installed >= fixed means
unaffected) while OSV last_affected is inclusive; update the handling in the
block that builds RangePair (refer to current_introduced, event.last_affected,
and RangePair.fixed) so that you convert the inclusive last_affected into an
exclusive bound (e.g., increment the last_affected version to the next logical
version/token) before assigning to fixed, or alternatively change the matching
logic that uses RangePair.fixed to treat it as inclusive; also add a
unit/integration test using an OSV advisory with a last_affected event to assert
versions equal to last_affected are considered affected.
Comment on lines
+9
to
+11
| const execute = async ({ logger: _logger, options, workspaceRoot }: Toolbox<Console, AdvisoriesPruneOptions>): Promise<void> => { | ||
| if (!workspaceRoot) { | ||
| throw new Error("Could not determine workspace root. Run this command inside a workspace."); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc comment for the exported function.
Per coding guidelines for TypeScript files, exported functions should include JSDoc comments with parameter types and return types.
📝 Proposed JSDoc addition
+/**
+ * Execute the advisories prune command to delete the local advisory database.
+ * Requires --force flag to proceed with the destructive operation.
+ *
+ * `@param` toolbox - The Cerebro toolbox containing logger, options, and workspaceRoot
+ * `@returns` Promise that resolves when the prune operation completes
+ * `@throws` {Error} When workspace root cannot be determined
+ */
const execute = async ({ logger: _logger, options, workspaceRoot }: Toolbox<Console, AdvisoriesPruneOptions>): Promise<void> => {As per coding guidelines, "Always include JSDoc comments for exported functions with parameter types and return types" for **/*.{ts,tsx,js,jsx} files.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const execute = async ({ logger: _logger, options, workspaceRoot }: Toolbox<Console, AdvisoriesPruneOptions>): Promise<void> => { | |
| if (!workspaceRoot) { | |
| throw new Error("Could not determine workspace root. Run this command inside a workspace."); | |
| /** | |
| * Execute the advisories prune command to delete the local advisory database. | |
| * Requires --force flag to proceed with the destructive operation. | |
| * | |
| * `@param` toolbox - The Cerebro toolbox containing logger, options, and workspaceRoot | |
| * `@returns` Promise that resolves when the prune operation completes | |
| * `@throws` {Error} When workspace root cannot be determined | |
| */ | |
| const execute = async ({ logger: _logger, options, workspaceRoot }: Toolbox<Console, AdvisoriesPruneOptions>): Promise<void> => { | |
| if (!workspaceRoot) { | |
| throw new Error("Could not determine workspace root. Run this command inside a workspace."); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis/src/commands/advisories/prune.ts` around lines 9 - 11,
Add a JSDoc comment above the exported async function execute describing the
function, its parameters (logger/_logger, options, workspaceRoot) with their
types (Toolbox<Console, AdvisoriesPruneOptions> and AdvisoriesPruneOptions
fields as needed) and the return type (Promise<void>); place it immediately
above the execute declaration so it documents execute and includes `@param`
entries for the Toolbox and its properties and an `@returns` Promise<void>.
Comment on lines
+8
to
+18
| const formatBytes = (bytes: number): string => { | ||
| if (bytes === 0) { | ||
| return "0 B"; | ||
| } | ||
|
|
||
| const units = ["B", "KB", "MB", "GB"]; | ||
| const exponent = Math.min(Math.floor(Math.log(bytes) / Math.log(1024)), units.length - 1); | ||
| const value = bytes / 1024 ** exponent; | ||
|
|
||
| return `${value.toFixed(value < 10 && exponent > 0 ? 1 : 0)} ${units[exponent]}`; | ||
| }; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add function description comment.
The formatBytes helper function lacks a comment describing its purpose. As per coding guidelines, add a comment at the start of each function describing what it does.
📝 Suggested comment
+/**
+ * Formats byte count to human-readable string with appropriate unit (B, KB, MB, GB).
+ */
const formatBytes = (bytes: number): string => {As per coding guidelines, functions should have a comment at the start describing what they do.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const formatBytes = (bytes: number): string => { | |
| if (bytes === 0) { | |
| return "0 B"; | |
| } | |
| const units = ["B", "KB", "MB", "GB"]; | |
| const exponent = Math.min(Math.floor(Math.log(bytes) / Math.log(1024)), units.length - 1); | |
| const value = bytes / 1024 ** exponent; | |
| return `${value.toFixed(value < 10 && exponent > 0 ? 1 : 0)} ${units[exponent]}`; | |
| }; | |
| /** | |
| * Formats byte count to human-readable string with appropriate unit (B, KB, MB, GB). | |
| */ | |
| const formatBytes = (bytes: number): string => { | |
| if (bytes === 0) { | |
| return "0 B"; | |
| } | |
| const units = ["B", "KB", "MB", "GB"]; | |
| const exponent = Math.min(Math.floor(Math.log(bytes) / Math.log(1024)), units.length - 1); | |
| const value = bytes / 1024 ** exponent; | |
| return `${value.toFixed(value < 10 && exponent > 0 ? 1 : 0)} ${units[exponent]}`; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis/src/commands/advisories/status.ts` around lines 8 - 18,
Add a short descriptive comment above the formatBytes function explaining its
purpose and behavior: state that formatBytes converts a numeric byte count into
a human-readable string using units B/KB/MB/GB, rounds to one decimal place for
values <10 KB/MB/GB and zero decimals otherwise, and handles the zero-case
returning "0 B"; place this comment immediately before the formatBytes
declaration to satisfy the coding guideline.
Comment on lines
+20
to
+54
| const formatRelative = (iso: string): string => { | ||
| if (!iso) { | ||
| return "never"; | ||
| } | ||
|
|
||
| const then = Date.parse(iso); | ||
|
|
||
| if (Number.isNaN(then)) { | ||
| return iso; | ||
| } | ||
|
|
||
| const deltaMs = Date.now() - then; | ||
|
|
||
| if (deltaMs < 0) { | ||
| return iso; | ||
| } | ||
|
|
||
| const minutes = Math.floor(deltaMs / 60_000); | ||
|
|
||
| if (minutes < 1) { | ||
| return "just now"; | ||
| } | ||
|
|
||
| if (minutes < 60) { | ||
| return `${minutes}m ago`; | ||
| } | ||
|
|
||
| const hours = Math.floor(minutes / 60); | ||
|
|
||
| if (hours < 24) { | ||
| return `${hours}h ago`; | ||
| } | ||
|
|
||
| return `${Math.floor(hours / 24)}d ago`; | ||
| }; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add function description comment.
The formatRelative helper function lacks a comment describing its purpose. As per coding guidelines, add a comment at the start of each function describing what it does.
📝 Suggested comment
+/**
+ * Converts ISO 8601 timestamp to human-readable relative time (e.g., "5m ago", "2h ago", "3d ago").
+ * Returns "never" for empty strings, "just now" for times less than 1 minute ago, or the original
+ * ISO string for invalid/future dates.
+ */
const formatRelative = (iso: string): string => {As per coding guidelines, functions should have a comment at the start describing what they do.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const formatRelative = (iso: string): string => { | |
| if (!iso) { | |
| return "never"; | |
| } | |
| const then = Date.parse(iso); | |
| if (Number.isNaN(then)) { | |
| return iso; | |
| } | |
| const deltaMs = Date.now() - then; | |
| if (deltaMs < 0) { | |
| return iso; | |
| } | |
| const minutes = Math.floor(deltaMs / 60_000); | |
| if (minutes < 1) { | |
| return "just now"; | |
| } | |
| if (minutes < 60) { | |
| return `${minutes}m ago`; | |
| } | |
| const hours = Math.floor(minutes / 60); | |
| if (hours < 24) { | |
| return `${hours}h ago`; | |
| } | |
| return `${Math.floor(hours / 24)}d ago`; | |
| }; | |
| /** | |
| * Converts ISO 8601 timestamp to human-readable relative time (e.g., "5m ago", "2h ago", "3d ago"). | |
| * Returns "never" for empty strings, "just now" for times less than 1 minute ago, or the original | |
| * ISO string for invalid/future dates. | |
| */ | |
| const formatRelative = (iso: string): string => { | |
| if (!iso) { | |
| return "never"; | |
| } | |
| const then = Date.parse(iso); | |
| if (Number.isNaN(then)) { | |
| return iso; | |
| } | |
| const deltaMs = Date.now() - then; | |
| if (deltaMs < 0) { | |
| return iso; | |
| } | |
| const minutes = Math.floor(deltaMs / 60_000); | |
| if (minutes < 1) { | |
| return "just now"; | |
| } | |
| if (minutes < 60) { | |
| return `${minutes}m ago`; | |
| } | |
| const hours = Math.floor(minutes / 60); | |
| if (hours < 24) { | |
| return `${hours}h ago`; | |
| } | |
| return `${Math.floor(hours / 24)}d ago`; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis/src/commands/advisories/status.ts` around lines 20 - 54,
Add a descriptive JSDoc-style comment above the formatRelative function
explaining its purpose (converts an ISO timestamp to a human-friendly relative
time string), the expected parameter (iso: string, may be empty or invalid), and
the possible return values ("never" for empty, the original string for
unparsable/future dates, "just now", "Xm ago", "Xh ago", or "Xd ago"). Reference
the function name formatRelative and describe its behavior for empty, NaN,
future timestamps and the minute/hour/day thresholds.
Comment on lines
+56
to
+118
| const execute = async ({ logger: _logger, options, workspaceRoot }: Toolbox<Console, AdvisoriesStatusOptions>): Promise<void> => { | ||
| if (!workspaceRoot) { | ||
| throw new Error("Could not determine workspace root. Run this command inside a workspace."); | ||
| } | ||
|
|
||
| const dbPath = options.db ?? resolveAdvisoryDbPath(workspaceRoot); | ||
| const status = await getAdvisoryStatus(workspaceRoot, dbPath); | ||
| const isJson = (options.format as string) === "json"; | ||
|
|
||
| if (isJson) { | ||
| process.stdout.write( | ||
| `${JSON.stringify( | ||
| { | ||
| dbPath, | ||
| ecosystems: status.ecosystems.map((e) => { | ||
| return { | ||
| advisoryCount: e.advisoryCount, | ||
| lastSyncIso: e.lastSyncIso, | ||
| manifestEtag: e.manifestEtag ?? null, | ||
| name: e.name, | ||
| }; | ||
| }), | ||
| exists: status.exists, | ||
| schemaVersion: status.schemaVersion, | ||
| sizeBytes: status.sizeBytes, | ||
| }, | ||
| undefined, | ||
| 2, | ||
| )}\n`, | ||
| ); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| pail.info(`DB: ${dbPath}`); | ||
|
|
||
| if (!status.exists) { | ||
| pail.warn(`No advisory DB yet. Run ${yellow("vis advisories sync")} to populate.`); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| pail.info(`Schema: v${status.schemaVersion} Size: ${formatBytes(status.sizeBytes)}`); | ||
|
|
||
| if (status.ecosystems.length === 0) { | ||
| pail.warn("DB exists but contains no ecosystems. Run `vis advisories sync`."); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| pail.info(""); | ||
| pail.info("Ecosystem Advisories Last sync ETag"); | ||
| pail.info(dim("────────── ────────── ───────────────── ──────")); | ||
|
|
||
| for (const eco of status.ecosystems) { | ||
| const name = eco.name.padEnd(10); | ||
| const count = eco.advisoryCount.toLocaleString().padStart(10); | ||
| const last = `${eco.lastSyncIso || "never"} (${formatRelative(eco.lastSyncIso)})`.padEnd(19); | ||
| const etag = eco.manifestEtag ?? dim("—"); | ||
|
|
||
| pail.info(`${green(name)} ${count} ${last} ${etag}`); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc comment for the command execute function.
While the execute function is indirectly exported (wrapped as advisoriesStatusExecute), it should have a JSDoc comment describing its purpose, parameters, and behavior. As per coding guidelines, exported functions should include JSDoc with parameter types and return types.
📚 Suggested JSDoc
+/**
+ * Executes the 'vis advisories status' command to report local OSV advisory DB status.
+ * Outputs either JSON format (when --format=json) or a human-readable table showing
+ * DB path, schema version, size, and per-ecosystem advisory counts with last sync times.
+ *
+ * `@param` toolbox - Command toolbox containing logger, options, and workspace root
+ */
const execute = async ({ logger: _logger, options, workspaceRoot }: Toolbox<Console, AdvisoriesStatusOptions>): Promise<void> => {As per coding guidelines, exported functions in TypeScript files should include JSDoc comments with parameter types and return types.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const execute = async ({ logger: _logger, options, workspaceRoot }: Toolbox<Console, AdvisoriesStatusOptions>): Promise<void> => { | |
| if (!workspaceRoot) { | |
| throw new Error("Could not determine workspace root. Run this command inside a workspace."); | |
| } | |
| const dbPath = options.db ?? resolveAdvisoryDbPath(workspaceRoot); | |
| const status = await getAdvisoryStatus(workspaceRoot, dbPath); | |
| const isJson = (options.format as string) === "json"; | |
| if (isJson) { | |
| process.stdout.write( | |
| `${JSON.stringify( | |
| { | |
| dbPath, | |
| ecosystems: status.ecosystems.map((e) => { | |
| return { | |
| advisoryCount: e.advisoryCount, | |
| lastSyncIso: e.lastSyncIso, | |
| manifestEtag: e.manifestEtag ?? null, | |
| name: e.name, | |
| }; | |
| }), | |
| exists: status.exists, | |
| schemaVersion: status.schemaVersion, | |
| sizeBytes: status.sizeBytes, | |
| }, | |
| undefined, | |
| 2, | |
| )}\n`, | |
| ); | |
| return; | |
| } | |
| pail.info(`DB: ${dbPath}`); | |
| if (!status.exists) { | |
| pail.warn(`No advisory DB yet. Run ${yellow("vis advisories sync")} to populate.`); | |
| return; | |
| } | |
| pail.info(`Schema: v${status.schemaVersion} Size: ${formatBytes(status.sizeBytes)}`); | |
| if (status.ecosystems.length === 0) { | |
| pail.warn("DB exists but contains no ecosystems. Run `vis advisories sync`."); | |
| return; | |
| } | |
| pail.info(""); | |
| pail.info("Ecosystem Advisories Last sync ETag"); | |
| pail.info(dim("────────── ────────── ───────────────── ──────")); | |
| for (const eco of status.ecosystems) { | |
| const name = eco.name.padEnd(10); | |
| const count = eco.advisoryCount.toLocaleString().padStart(10); | |
| const last = `${eco.lastSyncIso || "never"} (${formatRelative(eco.lastSyncIso)})`.padEnd(19); | |
| const etag = eco.manifestEtag ?? dim("—"); | |
| pail.info(`${green(name)} ${count} ${last} ${etag}`); | |
| } | |
| }; | |
| /** | |
| * Executes the 'vis advisories status' command to report local OSV advisory DB status. | |
| * Outputs either JSON format (when --format=json) or a human-readable table showing | |
| * DB path, schema version, size, and per-ecosystem advisory counts with last sync times. | |
| * | |
| * `@param` toolbox - Command toolbox containing logger, options, and workspace root | |
| */ | |
| const execute = async ({ logger: _logger, options, workspaceRoot }: Toolbox<Console, AdvisoriesStatusOptions>): Promise<void> => { | |
| if (!workspaceRoot) { | |
| throw new Error("Could not determine workspace root. Run this command inside a workspace."); | |
| } | |
| const dbPath = options.db ?? resolveAdvisoryDbPath(workspaceRoot); | |
| const status = await getAdvisoryStatus(workspaceRoot, dbPath); | |
| const isJson = (options.format as string) === "json"; | |
| if (isJson) { | |
| process.stdout.write( | |
| `${JSON.stringify( | |
| { | |
| dbPath, | |
| ecosystems: status.ecosystems.map((e) => { | |
| return { | |
| advisoryCount: e.advisoryCount, | |
| lastSyncIso: e.lastSyncIso, | |
| manifestEtag: e.manifestEtag ?? null, | |
| name: e.name, | |
| }; | |
| }), | |
| exists: status.exists, | |
| schemaVersion: status.schemaVersion, | |
| sizeBytes: status.sizeBytes, | |
| }, | |
| undefined, | |
| 2, | |
| )}\n`, | |
| ); | |
| return; | |
| } | |
| pail.info(`DB: ${dbPath}`); | |
| if (!status.exists) { | |
| pail.warn(`No advisory DB yet. Run ${yellow("vis advisories sync")} to populate.`); | |
| return; | |
| } | |
| pail.info(`Schema: v${status.schemaVersion} Size: ${formatBytes(status.sizeBytes)}`); | |
| if (status.ecosystems.length === 0) { | |
| pail.warn("DB exists but contains no ecosystems. Run `vis advisories sync`."); | |
| return; | |
| } | |
| pail.info(""); | |
| pail.info("Ecosystem Advisories Last sync ETag"); | |
| pail.info(dim("────────── ────────── ───────────────── ──────")); | |
| for (const eco of status.ecosystems) { | |
| const name = eco.name.padEnd(10); | |
| const count = eco.advisoryCount.toLocaleString().padStart(10); | |
| const last = `${eco.lastSyncIso || "never"} (${formatRelative(eco.lastSyncIso)})`.padEnd(19); | |
| const etag = eco.manifestEtag ?? dim("—"); | |
| pail.info(`${green(name)} ${count} ${last} ${etag}`); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis/src/commands/advisories/status.ts` around lines 56 -
118, Add a JSDoc block above the execute function describing its purpose (prints
advisory DB status in JSON or human-readable table), its parameter (toolbox:
Toolbox<Console, AdvisoriesStatusOptions> with logger, options, workspaceRoot),
and return type (Promise<void>), and note thrown errors (throws when
workspaceRoot is missing). Reference the function name execute (wrapped as
advisoriesStatusExecute) and briefly summarize behavior (computes dbPath, calls
getAdvisoryStatus, outputs JSON when options.format === "json" otherwise prints
formatted info) so the exported command has clear documentation.
Comment on lines
+25
to
+109
| const execute = async ({ logger: _logger, options, visConfig, workspaceRoot }: Toolbox<Console, AdvisoriesSyncOptions>): Promise<void> => { | ||
| if (!workspaceRoot) { | ||
| throw new Error("Could not determine workspace root. Run this command inside a workspace."); | ||
| } | ||
|
|
||
| const isJson = (options.format as string) === "json"; | ||
| const advisoriesConfig = readAdvisoriesConfig(visConfig); | ||
| const source = options.source ?? advisoriesConfig.source ?? DEFAULT_ADVISORY_SOURCE; | ||
| const ecosystems = parseEcosystems(options.ecosystem); | ||
|
|
||
| const tasks = ecosystems.map((eco) => { | ||
| return { id: eco, label: `Sync ${eco} advisories` }; | ||
| }); | ||
| const progress = startScanProgress(tasks, { live: !isJson }); | ||
|
|
||
| const results: { ecosystem: string; error?: string; result?: SyncResult }[] = []; | ||
|
|
||
| try { | ||
| for (const ecosystem of ecosystems) { | ||
| progress.start(ecosystem); | ||
| const startedAt = Date.now(); | ||
|
|
||
| try { | ||
| const result = await syncAdvisories({ | ||
| allowedHosts: advisoriesConfig.allowedHosts, | ||
| dbPath: options.db, | ||
| ecosystem, | ||
| force: Boolean(options.force), | ||
| source, | ||
| workspaceRoot, | ||
| }); | ||
|
|
||
| results.push({ ecosystem, result }); | ||
|
|
||
| if (result.upToDate) { | ||
| progress.finish(ecosystem, "ok", `up to date · ${formatDuration(Date.now() - startedAt)}`); | ||
| } else { | ||
| progress.finish(ecosystem, "ok", `${result.advisoriesIngested.toLocaleString()} advisories · ${formatDuration(result.durationMs)}`); | ||
| } | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
|
|
||
| results.push({ ecosystem, error: message }); | ||
| progress.finish(ecosystem, "error", message); | ||
| } | ||
| } | ||
| } finally { | ||
| progress.stop(); | ||
| } | ||
|
|
||
| if (isJson) { | ||
| const payload = { | ||
| ecosystems: results.map((r) => { | ||
| return { | ||
| advisoriesIngested: r.result?.advisoriesIngested ?? 0, | ||
| dbPath: r.result?.dbPath ?? null, | ||
| durationMs: r.result?.durationMs ?? 0, | ||
| ecosystem: r.ecosystem, | ||
| error: r.error ?? null, | ||
| upToDate: r.result?.upToDate ?? false, | ||
| }; | ||
| }), | ||
| source, | ||
| }; | ||
|
|
||
| process.stdout.write(`${JSON.stringify(payload, undefined, 2)}\n`); | ||
| } else { | ||
| const failed = results.filter((r) => r.error); | ||
| const succeeded = results.filter((r) => r.result); | ||
|
|
||
| if (succeeded.length > 0 && succeeded[0]?.result?.dbPath) { | ||
| pail.info(dim(`DB: ${succeeded[0].result.dbPath}`)); | ||
| } | ||
|
|
||
| if (failed.length === 0) { | ||
| pail.success(`Synced ${succeeded.length} ecosystem${succeeded.length === 1 ? "" : "s"}.`); | ||
| } else { | ||
| pail.error(`${failed.length} ecosystem${failed.length === 1 ? "" : "s"} failed to sync.`); | ||
| } | ||
| } | ||
|
|
||
| if (results.some((r) => r.error)) { | ||
| process.exitCode = 1; | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc comments for exported function.
The execute function and its export lack JSDoc documentation. As per coding guidelines, exported functions should include JSDoc comments with parameter types and return types.
📝 Suggested JSDoc documentation
+/**
+ * Executes the advisories sync command, downloading and ingesting OSV advisories
+ * for the specified ecosystems into a local SQLite database.
+ *
+ * `@param` toolbox - Command toolbox containing logger, options, config, and workspace root
+ * `@returns` Promise that resolves when sync completes
+ * `@throws` Error if workspace root cannot be determined
+ */
const execute = async ({ logger: _logger, options, visConfig, workspaceRoot }: Toolbox<Console, AdvisoriesSyncOptions>): Promise<void> => {As per coding guidelines, TypeScript files should include JSDoc comments for exported functions with parameter types and return types.
Also applies to: 113-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis/src/commands/advisories/sync.ts` around lines 25 - 109,
Add a JSDoc block immediately above the execute declaration describing the
function and its parameters and return type: document the execute async
function, its Toolbox<{ logger: Console, options: AdvisoriesSyncOptions,
visConfig, workspaceRoot }>/parameters (describe logger, options, visConfig,
workspaceRoot and any important option fields like format/source/db/force) using
`@param` tags referencing the existing types (Toolbox, AdvisoriesSyncOptions,
etc.), and add an `@returns` tag stating Promise<void>; ensure the JSDoc sits
directly above the const execute = async (...) declaration so the exported
function is properly documented.
…anner # Conflicts: # packages/tooling/vis/docs/commands/approve-builds.mdx # packages/tooling/vis/index.js # packages/tooling/vis/src/commands/approve-builds/handler.ts # packages/tooling/vis/src/commands/hook/dispatch.ts # packages/tooling/vis/src/config/types.ts # packages/tooling/vis/src/plugins/security-enforcement.ts # packages/tooling/vis/src/security/security.ts # packages/tooling/vis/src/util/docker.ts # pnpm-lock.yaml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/tooling/vis/src/security/drift.ts (1)
163-202:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrift report labels still reference legacy key names.
The drift extraction is updated correctly, but
formatDriftReportfurther down in this file (Lines 221, 225, 233, 238, 242) still emits user-facing labels likeallowBuilds — only in vis.config: …andminimumReleaseAge — vis.config: …. After the rename those keys no longer exist invis.config; users will be told to fix a setting whose name isn't in their config.Consider updating the labels to the new paths (
security.policies.install_scripts.allow,security.policies.first_seen.minutes,security.policies.first_seen.exclude) so the report matches the config schema users actually edit.📝 Proposed fix for the drift-report labels
if (report.allowBuilds) { if (report.allowBuilds.onlyInVis.length > 0) { - lines.push(` allowBuilds — only in vis.config: ${report.allowBuilds.onlyInVis.join(", ")}`); + lines.push(` security.policies.install_scripts.allow — only in vis.config: ${report.allowBuilds.onlyInVis.join(", ")}`); } if (report.allowBuilds.onlyInPm.length > 0) { - lines.push(` allowBuilds — only in ${report.packageManager} config: ${report.allowBuilds.onlyInPm.join(", ")}`); + lines.push(` security.policies.install_scripts.allow — only in ${report.packageManager} config: ${report.allowBuilds.onlyInPm.join(", ")}`); } } if (report.minReleaseAge) { const vis = report.minReleaseAge.vis === undefined ? "unset" : `${String(report.minReleaseAge.vis)} min`; const pm = report.minReleaseAge.pm === undefined ? "unset" : `${String(report.minReleaseAge.pm)} min`; - lines.push(` minimumReleaseAge — vis.config: ${vis}, ${report.packageManager}: ${pm}`); + lines.push(` security.policies.first_seen.minutes — vis.config: ${vis}, ${report.packageManager}: ${pm}`); } if (report.minReleaseAgeExcludes) { if (report.minReleaseAgeExcludes.onlyInVis.length > 0) { - lines.push(` minimumReleaseAgeExclude — only in vis.config: ${report.minReleaseAgeExcludes.onlyInVis.join(", ")}`); + lines.push(` security.policies.first_seen.exclude — only in vis.config: ${report.minReleaseAgeExcludes.onlyInVis.join(", ")}`); } if (report.minReleaseAgeExcludes.onlyInPm.length > 0) { - lines.push(` minimumReleaseAgeExclude — only in ${report.packageManager} config: ${report.minReleaseAgeExcludes.onlyInPm.join(", ")}`); + lines.push(` security.policies.first_seen.exclude — only in ${report.packageManager} config: ${report.minReleaseAgeExcludes.onlyInPm.join(", ")}`); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/security/drift.ts` around lines 163 - 202, The drift report still uses legacy labels; update formatDriftReport (and any helper that builds user-facing label strings) to reference the new config paths: replace occurrences referencing "allowBuilds" / "only in vis.config" with "security.policies.install_scripts.allow", replace "minimumReleaseAge" / "minimumReleaseAge — vis.config" with "security.policies.first_seen.minutes", and replace the excludes label with "security.policies.first_seen.exclude"; ensure the objects emitted (report.allowBuilds, report.minReleaseAge, report.minReleaseAgeExcludes) map their labels to these new path strings so the report text matches the actual vis.config keys users edit.packages/tooling/vis/src/config/config.ts (1)
596-630:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
defineConfigJSDoc examples still use the legacyallowBuilds/minimumReleaseAgekeys.After the breaking rename, both example blocks below the function declaration reference config shapes that are no longer valid:
security: { allowBuilds: { esbuild: true, "@prisma/client": true } }— should besecurity: { policies: { install_scripts: { allow: { esbuild: true, "@prisma/client": true } } } }security: { minimumReleaseAge: 1440, allowBuilds: { esbuild: true } }— should usepolicies.first_seen.minutesandpolicies.install_scripts.allowUsers who copy these snippets verbatim will get a config that silently does nothing (no type error if
VisConfig.securityallows excess properties via TS structural typing).As per coding guidelines: "Comments should describe purpose, not effect" and stale examples post-rename mislead consumers.📝 Proposed fix for the JSDoc examples
- * // vis.config.ts — minimal config, fully secured by defaults - * import { defineConfig } from "@visulima/vis/config"; - * - * export default defineConfig({ - * security: { - * allowBuilds: { - * esbuild: true, - * "@prisma/client": true, - * }, - * }, - * }); + * // vis.config.ts — minimal config, fully secured by defaults + * import { defineConfig } from "@visulima/vis/config"; + * + * export default defineConfig({ + * security: { + * policies: { + * install_scripts: { + * allow: { + * esbuild: true, + * "@prisma/client": true, + * }, + * }, + * }, + * }, + * });- * // vis.config.ts — override a default - * import { defineConfig } from "@visulima/vis/config"; - * - * export default defineConfig({ - * security: { - * // Relax cooldown to 24 hours instead of the default 14 days - * minimumReleaseAge: 1440, - * allowBuilds: { esbuild: true }, - * }, - * }); + * // vis.config.ts — override a default + * import { defineConfig } from "@visulima/vis/config"; + * + * export default defineConfig({ + * security: { + * policies: { + * // Relax cooldown to 24 hours instead of the default 14 days + * first_seen: { minutes: 1440 }, + * install_scripts: { allow: { esbuild: true } }, + * }, + * }, + * });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/config/config.ts` around lines 596 - 630, The JSDoc examples for defineConfig reference the old security keys (allowBuilds, minimumReleaseAge) which no longer match VisConfig; update both example blocks to the new security shape used by applyDefaults/VisConfig by replacing allowBuilds with security.policies.install_scripts.allow (e.g., security: { policies: { install_scripts: { allow: { esbuild: true, "@prisma/client": true } } } }) and replace minimumReleaseAge with the first_seen window (e.g., security: { policies: { first_seen: { minutes: 1440 } }, policies: { install_scripts: { allow: { esbuild: true } } } }) so the examples reflect the current policy keys and nesting.
🧹 Nitpick comments (6)
packages/tooling/vis/src/config/config.ts (2)
347-349: ⚡ Quick win
mergeVisConfigsshallow-mergessecurity— now riskier with deeply-nestedpolicies.
merged.security = { ...parent.security, ...child.security }overwrites the entirepoliciessub-tree when the child sets it at all. Pre-rename this was fine because the surface was flat; now anextendspreset that setssecurity.policies.install_scripts.allowwill be entirely discarded if the consumer's config sets evensecurity.policies.score.minimum. That subtle wipe is hard to debug for users who adopt presets.Consider mirroring the same nested merge used in
mergeSecurityDefaultshere — or at minimum deep-mergingsecurity.policies:if (parent.security || child.security) { - merged.security = { ...parent.security, ...child.security }; + merged.security = { + ...parent.security, + ...child.security, + policies: { ...parent.security?.policies, ...child.security?.policies }, + acceptedRisks: { ...parent.security?.acceptedRisks, ...child.security?.acceptedRisks }, + }; }Note: this still does a shallow merge inside individual policy blocks, which is consistent with
mergeSecurityDefaults. A full deep-merge would be the next iteration if needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/config/config.ts` around lines 347 - 349, mergeVisConfigs currently shallow-merges security which clobbers nested policies from parent when child sets any security field; update the merge to mirror mergeSecurityDefaults by deep-merging the policies subtree (e.g., merge parent.security and child.security but construct security.policies = { ...parent.security?.policies, ...child.security?.policies }) so that per-policy keys (like install_scripts.allow or score.minimum) are preserved; keep the existing shallow merge behavior inside each individual policy block to remain consistent with mergeSecurityDefaults.
55-79: 💤 Low valueDeep merge only covers two sub-policies — confirm that's intentional.
mergeSecurityDefaultsexplicitly deep-mergesinstall_scriptsandpublisher_changebecause those two have entries inSECURITY_DEFAULTS. That's correct today. However, if a future default is added to another sub-policy (first_seen,score,vulnerability, …), a contributor will need to remember to add another explicit deep-merge here or user-supplied values will start clobbering defaults.Consider a small generic merge so new policy defaults pick up the same semantics for free:
♻️ Optional refactor
const mergeSecurityDefaults = (security: VisConfig["security"]): VisConfig["security"] => { if (!security) { return { ...SECURITY_DEFAULTS }; } const defaultPolicies = SECURITY_DEFAULTS.policies ?? {}; const userPolicies = security.policies ?? {}; + const mergedPolicies: NonNullable<VisConfig["security"]>["policies"] = { ...defaultPolicies, ...userPolicies }; + + for (const key of Object.keys(defaultPolicies) as (keyof typeof defaultPolicies)[]) { + if (userPolicies[key]) { + mergedPolicies[key] = { ...defaultPolicies[key], ...userPolicies[key] } as never; + } + } return { ...SECURITY_DEFAULTS, ...security, - policies: { - ...defaultPolicies, - ...userPolicies, - install_scripts: { - ...defaultPolicies.install_scripts, - ...userPolicies.install_scripts, - }, - publisher_change: { - ...defaultPolicies.publisher_change, - ...userPolicies.publisher_change, - }, - }, + policies: mergedPolicies, }; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/config/config.ts` around lines 55 - 79, mergeSecurityDefaults currently deep-merges only install_scripts and publisher_change, so any new sub-policy added to SECURITY_DEFAULTS will be clobbered by user values; update mergeSecurityDefaults to generically deep-merge all keys under policies by iterating the union of Object.keys(SECURITY_DEFAULTS.policies) and Object.keys(security.policies) and for each key produce a merged object of default and user (falling back to the user value if it’s not an object), while leaving the rest of the top-level merge logic intact; reference mergeSecurityDefaults, SECURITY_DEFAULTS.policies and security.policies when locating the code to change.packages/tooling/vis/src/config/types.ts (4)
20-28: 💤 Low valuePublic API uses snake_case keys — confirm this is the intended config surface.
PolicyNameand the correspondingsecurity.policies.*keys (first_seen,install_scripts,publisher_change,unexpected_deps) use snake_case identifiers. This is fine when it maps 1:1 to Socket.dev's policy classifications, but it diverges from the rest ofVisConfig, which is camelCase (acceptedRisks,allowBins,blockExoticSubdeps,pinVersions,typosquatAllowlist, etc.). Since this is a breaking config rename anyway, please confirm the casing decision now — switching later would be another breaking change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/config/types.ts` around lines 20 - 28, PolicyName and the security.policies keys use snake_case while the rest of VisConfig uses camelCase; decide and apply a consistent casing now (either convert PolicyName to camelCase variants like firstSeen/installScripts/publisherChange/unexpectedDeps and update all uses, tests and any (de)serialization/validation logic, or keep snake_case but add an explicit mapping layer between external config and internal camelCase fields and document it). Locate the PolicyName type in types.ts and update its union members if converting, or implement the mapping in the config parsing/validation code that consumes security.policies so keys are normalized to the project's camelCase fields (and update related tests/docs accordingly).
998-1080: 💤 Low value
security.auditshape: a couple of small consistency items.
advisories.sourcedocuments the default (https://osv-vulnerabilities.storage.googleapis.com) in prose but omits a@defaultJSDoc tag — every other field in this subtree has one. Adding@default "https://osv-vulnerabilities.storage.googleapis.com"keeps the IDE hint surface consistent.advisories.allowedHostsexample is["mirror.corp.example.com"](bare host), and the prose forsourcesays it "must behttps://and resolve to a host inallowedHosts". Confirm the validator compares bare hostnames (not full URLs) — if a user follows the example but the validator expectshttps://mirror.corp.example.com, the override will silently fall back to the default and be hard to debug. Worth a unit test on the host-matching path if one isn't already inaudit.advisories.sourcevalidation.apply.transitive.enabledis the only knob underapply; the surrounding doc onaudit.applyalready lists the CLI-side--yesrule. If you anticipate adding adirectcounterpart soon, fine to leave; otherwise flattening toapply?: { transitiveEnabled?: boolean }would simplify the public surface.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/config/types.ts` around lines 998 - 1080, Add a `@default` JSDoc to audit.advisories.source documenting "https://osv-vulnerabilities.storage.googleapis.com"; verify/fix the validator that checks audit.advisories.source so it extracts the hostname from the URL (require https and then compare only the host portion against advisories.allowedHosts entries) rather than comparing full URLs vs bare hosts, and add a unit test covering a mirror entry like "mirror.corp.example.com" vs "https://mirror.corp.example.com" to prevent silent fallback; finally, decide whether to simplify the shape by renaming audit.apply.transitive.enabled to audit.apply.transitiveEnabled (or leave as-is if you expect more keys under apply) and update the type/comment accordingly (refer to the types audit.advisories.allowedHosts, audit.advisories.source, and audit.apply.transitive.enabled).
1213-1220: 💤 Low value
malware.modedefault doc is ambiguous.
@default { mode: "block" } when Socket is enabled, off otherwise— the actual default depends on a sibling field (security.socket.enabled). This kind of cross-field default is fine but should be implemented (and tested) consistently in the consumers; otherwise users disabling Socket may still see malware-policy blocks (or vice versa). Worth a one-line note on which file resolves this default, and a test that flipssocket.enabledand asserts the resultingmalware.mode.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/config/types.ts` around lines 1213 - 1220, The doc for malware.mode is ambiguous because its default depends on security.socket.enabled; update the comment for malware.mode to state the cross-field default behavior explicitly and include a one-line note pointing to the config default resolver used to implement this behavior (the config normalization/merge code that sets cross-field defaults), and add a unit/integration test that toggles security.socket.enabled and asserts malware.mode resolves to "block" when socket.enabled is true and "off" when socket.enabled is false to ensure consumers observe the intended default.
960-984: 💤 Low value
acceptedRisksvalue shape: tightenexpiresAtsemantics andacceptedScorerange.A couple of points worth confirming before this lands as the new stable surface:
expiresAtis typed asstringwith the docstring "ISO 8601 date (or datetime)". There is no compile-time or runtime constraint here, so a typo or epoch number serialized as string will silently parse toNaNand the entry will effectively never expire (or always be expired, depending on how the consumer compares). Worth either documenting the validation behavior or having the consuming code reject unparseable values explicitly.acceptedScoreisnumberbut doc'd as the Socket overall score; elsewherepolicies.score.minimumis documented as0–1. Consider mirroring that bound in the doc for clarity, since callers will compare againstpolicies.score.minimum.Not blocking — both are docs/consumer-side concerns rather than type defects.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/config/types.ts` around lines 960 - 984, The acceptedRisks shape allows unvalidated expiresAt strings and unconstrained acceptedScore values; update the surface by (1) tightening docs and adding runtime validation where acceptedRisks are parsed/constructed: ensure expiresAt (in acceptedRisks entries) is parsed to a valid Date (reject or throw if Date is invalid/NaN) and ensure acceptedScore is within [0,1] (reject/normalize if outside), and (2) update the TypeScript/JSDoc for acceptedRisks.acceptedScore to indicate the 0–1 range and for expiresAt to state that consumers will validate ISO8601 and reject unparseable values; add/rename a validation helper (e.g., validateAcceptedRisk or parseAcceptedRisks) used by any loader/constructor that builds acceptedRisks to perform these checks referencing acceptedRisks, expiresAt, and acceptedScore (and keep PolicyName unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/tooling/vis/index.js`:
- Around line 735-737: Add a new TypeScript declaration file
packages/tooling/vis/index.d.ts that declares the native exports listed in
index.js (including advisoriesIngest, advisoriesQuery, advisoriesStatus and the
other functions exported from the native binding) and ensure package.json
"types" points to ./index.d.ts; declare each exported function (e.g.,
advisoriesIngest, advisoriesQuery, advisoriesStatus) with appropriate parameter
and return types (use specific types if known, otherwise use sensible broader
types such as (args: any) => Promise<any> or (args: any) => any) and export them
with export function/... or export = for the module shape to match how index.js
exposes the native binding so TypeScript consumers can import the native
functions without errors.
In `@packages/tooling/vis/src/config/config-writer.ts`:
- Line 57: The current regex (allowBlockMatch) in config-writer.ts only matches
the first "allow: {}" anywhere and the other regexes targeting
"install_scripts"/"policies" are too loose, which can update similarly named
keys outside the security tree; update the implementation to operate on the
actual exported config AST (parse the file and mutate the defineConfig/export
default object) and specifically locate security.policies.install_scripts.allow,
or if you must keep a regex approach, tighten the patterns to anchor the full
nesting chain (security → policies → install_scripts → allow) before performing
insertion/replacement so only security.policies.install_scripts.allow is
modified (replace usages of allowBlockMatch and the install_scripts/policies
matches accordingly).
In `@packages/tooling/vis/src/config/types.ts`:
- Around line 1139-1319: The JSDoc claims policies.score.minimum defaults to 0.4
but that fallback is only applied in add/handler.ts; update the code to make the
documented default actually used: modify buildSocketOptions to set scoreMinimum
(or socketOptions.minimumScore) to DEFAULT_LOW_SCORE_THRESHOLD when undefined,
and then ensure all callers/handlers (update, check, doctor, audit, analyze,
plugins/security-enforcement) and display helpers (e.g., formatSecurityOverview)
read and use socketOptions.minimumScore instead of the hardcoded
DEFAULT_LOW_SCORE_THRESHOLD so the config value flows everywhere; alternatively,
if you prefer to remove the misleading doc, update the JSDoc on
policies.score.minimum to remove the `@default` 0.4 claim and document that
handlers may default elsewhere.
---
Outside diff comments:
In `@packages/tooling/vis/src/config/config.ts`:
- Around line 596-630: The JSDoc examples for defineConfig reference the old
security keys (allowBuilds, minimumReleaseAge) which no longer match VisConfig;
update both example blocks to the new security shape used by
applyDefaults/VisConfig by replacing allowBuilds with
security.policies.install_scripts.allow (e.g., security: { policies: {
install_scripts: { allow: { esbuild: true, "@prisma/client": true } } } }) and
replace minimumReleaseAge with the first_seen window (e.g., security: {
policies: { first_seen: { minutes: 1440 } }, policies: { install_scripts: {
allow: { esbuild: true } } } }) so the examples reflect the current policy keys
and nesting.
In `@packages/tooling/vis/src/security/drift.ts`:
- Around line 163-202: The drift report still uses legacy labels; update
formatDriftReport (and any helper that builds user-facing label strings) to
reference the new config paths: replace occurrences referencing "allowBuilds" /
"only in vis.config" with "security.policies.install_scripts.allow", replace
"minimumReleaseAge" / "minimumReleaseAge — vis.config" with
"security.policies.first_seen.minutes", and replace the excludes label with
"security.policies.first_seen.exclude"; ensure the objects emitted
(report.allowBuilds, report.minReleaseAge, report.minReleaseAgeExcludes) map
their labels to these new path strings so the report text matches the actual
vis.config keys users edit.
---
Nitpick comments:
In `@packages/tooling/vis/src/config/config.ts`:
- Around line 347-349: mergeVisConfigs currently shallow-merges security which
clobbers nested policies from parent when child sets any security field; update
the merge to mirror mergeSecurityDefaults by deep-merging the policies subtree
(e.g., merge parent.security and child.security but construct security.policies
= { ...parent.security?.policies, ...child.security?.policies }) so that
per-policy keys (like install_scripts.allow or score.minimum) are preserved;
keep the existing shallow merge behavior inside each individual policy block to
remain consistent with mergeSecurityDefaults.
- Around line 55-79: mergeSecurityDefaults currently deep-merges only
install_scripts and publisher_change, so any new sub-policy added to
SECURITY_DEFAULTS will be clobbered by user values; update mergeSecurityDefaults
to generically deep-merge all keys under policies by iterating the union of
Object.keys(SECURITY_DEFAULTS.policies) and Object.keys(security.policies) and
for each key produce a merged object of default and user (falling back to the
user value if it’s not an object), while leaving the rest of the top-level merge
logic intact; reference mergeSecurityDefaults, SECURITY_DEFAULTS.policies and
security.policies when locating the code to change.
In `@packages/tooling/vis/src/config/types.ts`:
- Around line 20-28: PolicyName and the security.policies keys use snake_case
while the rest of VisConfig uses camelCase; decide and apply a consistent casing
now (either convert PolicyName to camelCase variants like
firstSeen/installScripts/publisherChange/unexpectedDeps and update all uses,
tests and any (de)serialization/validation logic, or keep snake_case but add an
explicit mapping layer between external config and internal camelCase fields and
document it). Locate the PolicyName type in types.ts and update its union
members if converting, or implement the mapping in the config parsing/validation
code that consumes security.policies so keys are normalized to the project's
camelCase fields (and update related tests/docs accordingly).
- Around line 998-1080: Add a `@default` JSDoc to audit.advisories.source
documenting "https://osv-vulnerabilities.storage.googleapis.com"; verify/fix the
validator that checks audit.advisories.source so it extracts the hostname from
the URL (require https and then compare only the host portion against
advisories.allowedHosts entries) rather than comparing full URLs vs bare hosts,
and add a unit test covering a mirror entry like "mirror.corp.example.com" vs
"https://mirror.corp.example.com" to prevent silent fallback; finally, decide
whether to simplify the shape by renaming audit.apply.transitive.enabled to
audit.apply.transitiveEnabled (or leave as-is if you expect more keys under
apply) and update the type/comment accordingly (refer to the types
audit.advisories.allowedHosts, audit.advisories.source, and
audit.apply.transitive.enabled).
- Around line 1213-1220: The doc for malware.mode is ambiguous because its
default depends on security.socket.enabled; update the comment for malware.mode
to state the cross-field default behavior explicitly and include a one-line note
pointing to the config default resolver used to implement this behavior (the
config normalization/merge code that sets cross-field defaults), and add a
unit/integration test that toggles security.socket.enabled and asserts
malware.mode resolves to "block" when socket.enabled is true and "off" when
socket.enabled is false to ensure consumers observe the intended default.
- Around line 960-984: The acceptedRisks shape allows unvalidated expiresAt
strings and unconstrained acceptedScore values; update the surface by (1)
tightening docs and adding runtime validation where acceptedRisks are
parsed/constructed: ensure expiresAt (in acceptedRisks entries) is parsed to a
valid Date (reject or throw if Date is invalid/NaN) and ensure acceptedScore is
within [0,1] (reject/normalize if outside), and (2) update the TypeScript/JSDoc
for acceptedRisks.acceptedScore to indicate the 0–1 range and for expiresAt to
state that consumers will validate ISO8601 and reject unparseable values;
add/rename a validation helper (e.g., validateAcceptedRisk or
parseAcceptedRisks) used by any loader/constructor that builds acceptedRisks to
perform these checks referencing acceptedRisks, expiresAt, and acceptedScore
(and keep PolicyName unchanged).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca2f87f8-9a41-4502-b82b-a513557dfda5
⛔ Files ignored due to path filters (4)
packages/tooling/vis/__tests__/security/config-writer.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/drift-check.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/script-security.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by none
📒 Files selected for processing (25)
packages/tooling/vis/docs/commands/approve-builds.mdxpackages/tooling/vis/docs/meta.jsonpackages/tooling/vis/index.jspackages/tooling/vis/package.jsonpackages/tooling/vis/src/bin.tspackages/tooling/vis/src/commands/add/handler.tspackages/tooling/vis/src/commands/approve-builds/handler.tspackages/tooling/vis/src/commands/approve-builds/index.tspackages/tooling/vis/src/commands/init/handler.tspackages/tooling/vis/src/commands/install/handler.tspackages/tooling/vis/src/commands/security/index.tspackages/tooling/vis/src/commands/security/list.tspackages/tooling/vis/src/commands/security/run.tspackages/tooling/vis/src/commands/security/sync.tspackages/tooling/vis/src/commands/update/handler.tspackages/tooling/vis/src/config/config-writer.tspackages/tooling/vis/src/config/config.tspackages/tooling/vis/src/config/types.tspackages/tooling/vis/src/plugins/security-enforcement.tspackages/tooling/vis/src/security/config-check.tspackages/tooling/vis/src/security/drift.tspackages/tooling/vis/src/security/enforcement.tspackages/tooling/vis/src/security/min-release-age.tspackages/tooling/vis/src/security/native-config-sync.tspackages/tooling/vis/src/security/security.ts
✅ Files skipped from review due to trivial changes (6)
- packages/tooling/vis/src/security/native-config-sync.ts
- packages/tooling/vis/src/security/security.ts
- packages/tooling/vis/src/commands/security/index.ts
- packages/tooling/vis/src/security/min-release-age.ts
- packages/tooling/vis/src/commands/install/handler.ts
- packages/tooling/vis/docs/meta.json
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/tooling/vis/src/bin.ts
- packages/tooling/vis/package.json
- packages/tooling/vis/src/commands/add/handler.ts
- packages/tooling/vis/src/commands/approve-builds/handler.ts
- packages/tooling/vis/src/commands/init/handler.ts
Comment on lines
+735
to
+737
| advisoriesIngest, | ||
| advisoriesQuery, | ||
| advisoriesStatus, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for TypeScript definitions of the new advisories exports
# Look for the .d.ts file in the same directory
fd -e d.ts -d 1 . packages/tooling/vis/
# Search for the new function declarations in TypeScript definition files
rg -n "advisoriesIngest|advisoriesQuery|advisoriesStatus" packages/tooling/vis/ --glob "*.d.ts" -A 2Repository: visulima/visulima
Length of output: 43
🏁 Script executed:
#!/bin/bash
# More comprehensive search for TypeScript definitions
# 1. Check if index.d.ts exists at the root of packages/tooling/vis/
ls -la packages/tooling/vis/ | grep -E '\.d\.ts|\.js'
# 2. Search for .d.ts files anywhere in the vis package (no depth limit)
find packages/tooling/vis -name "*.d.ts" -type f
# 3. Check the package.json to understand the structure
cat packages/tooling/vis/package.json | head -30
# 4. Look for the three functions in index.js to confirm they exist
rg -n "advisoriesIngest|advisoriesQuery|advisoriesStatus" packages/tooling/vis/index.jsRepository: visulima/visulima
Length of output: 1614
🏁 Script executed:
#!/bin/bash
# Check for all .d.ts files and verify package.json exports configuration
# 1. Find ALL .d.ts files in the vis package
find packages/tooling/vis -name "*.d.ts" -type f
# 2. Check the full package.json, especially types/exports/main fields
cat packages/tooling/vis/package.json | grep -A 10 -E '"types"|"exports"|"main"'
# 3. Search for the three new functions in ALL .d.ts files
rg -r "advisoriesIngest|advisoriesQuery|advisoriesStatus" packages/tooling/vis --glob "*.d.ts" -B 2 -A 2
# 4. Check if there's a dist directory with .d.ts files
ls -la packages/tooling/vis/dist/ 2>/dev/null || echo "No dist directory"
# 5. Look at the structure of the cerebro-augment.d.ts file
cat packages/tooling/vis/src/cerebro-augment.d.tsRepository: visulima/visulima
Length of output: 1087
Create missing index.d.ts TypeScript definition file for native exports.
The package.json declares that TypeScript types for the ./native export should exist at ./index.d.ts, but this file is missing. The three new exports (advisoriesIngest, advisoriesQuery, advisoriesStatus) at lines 735-737 and 763-765 have no type definitions, breaking TypeScript support for this public API.
Generate packages/tooling/vis/index.d.ts with proper type declarations for all exported functions from the native binding.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis/index.js` around lines 735 - 737, Add a new TypeScript
declaration file packages/tooling/vis/index.d.ts that declares the native
exports listed in index.js (including advisoriesIngest, advisoriesQuery,
advisoriesStatus and the other functions exported from the native binding) and
ensure package.json "types" points to ./index.d.ts; declare each exported
function (e.g., advisoriesIngest, advisoriesQuery, advisoriesStatus) with
appropriate parameter and return types (use specific types if known, otherwise
use sensible broader types such as (args: any) => Promise<any> or (args: any) =>
any) and export them with export function/... or export = for the module shape
to match how index.js exposes the native binding so TypeScript consumers can
import the native functions without errors.
- Add JSDoc to exported items across hook builtins, advisories, audit, docker - Document OSV `last_affected` inclusive-upper semantics with a focused test - Annotate NAPI u64→u32 truncations with JS Number range rationale - Switch vis-mcp tool payloads to zod schemas with `.catchall(z.unknown())` so unknown CLI fields stay forward-compatible - Fence RFC code blocks with explicit languages and tidy hook-command formatting (multi-line type, implicit-return arrows) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
- Drift report labels now reference `security.policies.*` paths so the hint matches the keys users edit - `defineConfig` JSDoc examples use the new `security.policies` shape - `mergeSecurityDefaults` deep-merges every defaulted sub-policy generically; `mergeVisConfigs` deep-merges `policies` and `acceptedRisks` so presets aren't clobbered - Config-writer scopes its `allow:` match to follow an `install_scripts:` opener - JSDoc clarifications for `audit.advisories.source` default, `acceptedRisks.expiresAt`/`acceptedScore` ranges, `malware.mode` cross-field default, and the current `policies.score.minimum` wiring gap Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…re score.minimum - Rename `PolicyName` union to camelCase (`firstSeen`, `installScripts`, `publisherChange`, `unexpectedDeps`). Updates every consumer in src/, test fixtures, schemas, and docs. JSON schema regenerated. - Add `types` mapping to the `#native` package.json import so TypeScript resolves `index.d.ts` when consumers import the alias. - Thread `socketOptions.minimumScore` through `audit`, `doctor`, `check`, `update`, `add`, `formatSecurityOverview`, `formatSummary`, and `applyFilter`. `buildSocketOptions` now resolves the effective minimum once (from `security.policies.score.minimum` or `DEFAULT_LOW_SCORE_THRESHOLD`) so every consumer sees the same value instead of comparing scores against the hard-coded constant. - Update the JSDoc on `policies.score.minimum` to reflect the new wiring. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Introduce a unified `evaluatePolicies()` engine under `src/security/policies/` with four offline-clean modules: license, install_scripts, vulnerability, and unexpected_deps (baseline mode). Each policy emits PolicyDecisions (block/warn/info) keyed by package, with per-policy accepted-risk scoping and expiresAt support reused from the shared matcher. Wires the engine into `vis audit`: a new `--policies <names>` flag (comma-list, `all`, or `none`) narrows evaluation. Block-severity decisions feed into `--exit-code` and `--fail-on`. JSON output gains `policies[]` + `summary.policyBlocks`. SARIF and HTML formatters render policy decisions alongside vulnerabilities; CSAF and CycloneDX-VEX are intentionally left untouched (vuln-specific data models). Adds `readNodeModulesManifests()` to walk `node_modules/` (including the pnpm `.pnpm/` content-addressed store) and surface license + scripts + maintainers metadata for the offline policies. 33 new unit + integration tests; full vitest suite (3582/3582) green; `tsc --noEmit` clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- applyExitGate's fallthrough call to applyFailOnGate was dropping the policyDecisions argument, so `vis audit --format sarif --fail-on high` exited 0 when the only signal was a block-severity policy decision. Forward the argument. - Unknown --policies tokens were silently swallowed when format was json/sarif/csaf/cyclonedx-vex, so a typoed CI invocation reduced enforcement with no log. Always emit the warning to stderr and surface the tokens in JSON output as `warnings[]`. - Vulnerability-policy block decisions whose advisory was masked by --severity used to exit 1 with no visible reason. Surface those decisions in the human-readable "Policy Decisions" section so the gate is always traceable. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
Author
Self-audit of bb50aa8 → fa5386a (Phase 2 policy engine)Ran Blockers (fixed)
Significant suggestions (logged for follow-up, not addressed in this push)
Nitpicks (logged)
Verification
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
packages/tooling/vis/schemas/vis-config.schema.json (1)
1022-1055:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the
firstSeenschema default here.Line 1054 still publishes a scalar default for an object schema, and it also disagrees with runtime behavior:
config.tsintentionally leavessecurity.policies.firstSeenunset unless the user configures it orvis initwrites it. Keeping a schema default here makes schema-driven tooling materialize a policy that runtime treats as “no opinion”.Suggested fix
- "default": 0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/schemas/vis-config.schema.json` around lines 1022 - 1055, The schema defines a scalar default for the object property "firstSeen" (default: 0) which contradicts runtime behavior (security.policies.firstSeen is left unset unless configured); remove the "default": 0 entry from the "firstSeen" object schema in vis-config.schema.json so the schema does not materialize a default policy and aligns with config.ts handling of security.policies.firstSeen.
🧹 Nitpick comments (4)
packages/tooling/vis/src/commands/hook/builtins/trailing-whitespace.ts (1)
7-7: ⚡ Quick winRewrite the inline comment to explain intent, not just byte labels.
Current comment lists characters but not why
\ris included. A purpose-oriented comment is clearer for future maintenance.Proposed diff
-// TAB, VT, FF, CR, SP +// Bytes treated as trailing whitespace when trimming line content.As per coding guidelines, "Comments should describe purpose, not effect".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/hook/builtins/trailing-whitespace.ts` at line 7, Replace the terse inline comment "// TAB, VT, FF, CR, SP" with a purpose-focused comment that explains why those specific control characters are treated as trailing-whitespace (e.g., "Treat horizontal tab, vertical tab, form feed, carriage return and space as trailing whitespace so that CR from CRLF line endings and other control whitespace are detected/trimmed") and mention the rationale for including '\r' (to catch CR in CRLF pairs). Update the comment adjacent to the trailing-whitespace logic in trailing-whitespace.ts to clearly state intent rather than listing byte labels.packages/tooling/vis/src/commands/hook/builtins/mixed-line-ending.ts (1)
29-33: ⚡ Quick winAdd explicit JSDoc types to exported function tags.
The exported function JSDoc has descriptions, but the tags are missing explicit type annotations required by repo rules.
Proposed diff
- * `@param` files Files (relative to `context.root`) to process. - * `@param` args Raw CLI args passed through from the hook config. - * `@param` context Builtin context with logger and workspace root. - * `@returns` Exit code: `0` on no-op, `1` on fixed/found mixed endings, `2` on bad args. + * `@param` {ReadonlyArray<string>} files Files (relative to `context.root`) to process. + * `@param` {ReadonlyArray<string>} args Raw CLI args passed through from the hook config. + * `@param` {BuiltinContext} context Builtin context with logger and workspace root. + * `@returns` {number} Exit code: `0` on no-op, `1` on fixed/found mixed endings, `2` on bad args.As per coding guidelines, "Always include JSDoc comments for exported functions with parameter types and return types".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/hook/builtins/mixed-line-ending.ts` around lines 29 - 33, The exported function's JSDoc is missing explicit types for its parameters and return; update the JSDoc above the exported function that accepts files, args, and context to include explicit types that match the TypeScript signature (for example add `@param` {string[]} files, `@param` {string[]} args, `@param` {YourContextType} context and an explicit `@returns` {number} or {`@link` Promise<number>} if the function is async), ensuring the type name used for context matches the actual type in the file.packages/tooling/vis/docs/commands/add.mdx (1)
54-54: ⚡ Quick winClarify the
--run-scriptsdescription.The current wording creates logical confusion: if
--run-scripts"opts out of the universal block-by-default policy," the phrase "allowlisted packages still run viasecurity.policies.installScripts.allow" suggests the allowlist applies even when opting out, which appears contradictory.Consider clarifying the relationship:
- Does
--run-scriptsenable scripts for all packages in this run (overriding both the block-by-default policy and the allowlist)?- Or does it only affect non-allowlisted packages?
Suggested clearer wording
If
--run-scriptsenables all scripts for the run:-| `--run-scripts` | | `false` | Run lifecycle scripts (opts out of the universal block-by-default policy; allowlisted packages still run via `security.policies.installScripts.allow`) | +| `--run-scripts` | | `false` | Run lifecycle scripts for this install (opts out of the block-by-default policy; persistent allowlist: `security.policies.installScripts.allow`) |If the allowlist continues to apply separately:
-| `--run-scripts` | | `false` | Run lifecycle scripts (opts out of the universal block-by-default policy; allowlisted packages still run via `security.policies.installScripts.allow`) | +| `--run-scripts` | | `false` | Run lifecycle scripts for non-allowlisted packages (one-time opt-out; allowlist at `security.policies.installScripts.allow`) |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/docs/commands/add.mdx` at line 54, Update the `--run-scripts` description to remove the logical ambiguity by explicitly stating its precedence relative to the block-by-default policy and the allowlist: clarify whether `--run-scripts` globally enables lifecycle scripts for the entire run (overriding the universal block-by-default policy and causing scripts to run for all packages) or whether it only enables scripts for non-allowlisted packages while the allowlist governed by `security.policies.installScripts.allow` still controls allowlisted behavior; edit the table row containing `--run-scripts` so it clearly states the chosen semantics (e.g., "Enable lifecycle scripts for all packages in this run, overriding the default block policy" OR "Enable scripts for packages that are not already allowlisted; allowlisted packages still follow security.policies.installScripts.allow") and pick one consistent phrasing.packages/tooling/vis/src/report/audit-html.ts (1)
43-44: 💤 Low valueManual HTML sanitization is acceptable here but consider the tradeoff.
The static analysis tool flagged manual HTML escaping as potentially error-prone. While your implementation covers the standard set (
&,<,>,",') and is sufficient for this local-report context, a dedicated sanitization library (likesanitize-htmlorDOMPurify) would provide defense-in-depth against edge cases.Since the report is generated locally (not served over the web) and the escaping covers the necessary cases for this specific output, the current implementation is acceptable. The risk is mitigated by the usage context.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/report/audit-html.ts` around lines 43 - 44, The manual HTML escaping in the escapeHtml function currently handles &, <, >, " and ' but static analysis flagged it as potentially error-prone; either (A) keep the implementation but add a clear comment above escapeHtml explaining the local-only report context and why this minimal escaping is sufficient (and add unit tests for edge cases like null/empty/emoji/emoji combining sequences), or (B) replace the manual approach by importing a vetted sanitizer (e.g. sanitize-html or DOMPurify), use its safeEscape/escape function inside escapeHtml (preserve the escapeHtml symbol name), and update tests and package.json accordingly so the codebase uses a library for defense-in-depth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/tooling/vis-mcp/src/tools/advisory-status.ts`:
- Around line 40-43: The schema and branching silently allow and ignore empty db
overrides; update the zod schema for the db field (currently
z.string().optional()) to reject empty strings (e.g.,
z.string().min(1).optional() or a .refine(...) non-empty check) and change the
runtime check that uses input.db (e.g., the if (input.db) branch) to test
explicitly for undefined (if (input.db !== undefined)) so an explicit
empty-string validation error is raised and a deliberate override of an empty
value is not treated as “absent”.
In `@packages/tooling/vis/schemas/vis-config.schema.json`:
- Around line 1252-1253: The description incorrectly states that omitting a
policy sub-block makes the policy inactive; update it to reflect runtime
defaults by either (a) changing the text to note that some policies can be
enabled by runtime defaults (SECURITY_DEFAULTS) even if their blocks are omitted
— specifically call out policies.installScripts.strict and
policies.publisherChange — or (b) make the schema defaults align with
SECURITY_DEFAULTS so the schema-generated docs match runtime (ensure
SECURITY_DEFAULTS and the schema keys policies.installScripts.strict and
policies.publisherChange are consistent).
In `@packages/tooling/vis/src/config/config.ts`:
- Around line 349-359: The current merge for security in the block setting
merged.security shallow-merges the top-level policies object so when both parent
and child define the same policy key (e.g., security.policies.installScripts)
the child's object replaces the parent's and inherited fields are lost; change
the merge to deep-merge each policy body like mergeSecurityDefaults does by
iterating over the union of policy keys and merging
parent.security.policies[policy] with child.security.policies[policy], while
still merging acceptedRisks shallowly as shown (i.e., produce merged.security
with merged.acceptedRisks and merged.policies where each policy value is a
merged object of parent and child policy bodies).
In `@packages/tooling/vis/src/report/audit-html.ts`:
- Around line 133-345: Add a JSDoc block above the exported function
emitAuditHtml describing the function, its single parameter (options:
AuditHtmlEmitOptions) and the return type (string); include descriptions for the
main properties used (e.g., options.now, options.findings, options.tool,
options.packagesScanned, options.policyDecisions) and note that it returns the
generated HTML string. Ensure the JSDoc uses TypeScript-style type annotations
and is placed immediately above the export so linters/IDE pick it up.
In `@packages/tooling/vis/src/report/sarif.ts`:
- Around line 151-292: The exported function emitSarif lacks a JSDoc block: add
a JSDoc comment immediately above the export const emitSarif declaration that
documents the parameter (options: SarifEmitOptions) and the return type
(SarifLog), briefly describes the function purpose, and notes important option
fields used (e.g., findings, policyDecisions, workspaceRoot, artifactUri, tool)
so IDEs and docs pick up types and intent; ensure the JSDoc includes `@param`
{SarifEmitOptions} options and `@returns` {SarifLog}.
---
Duplicate comments:
In `@packages/tooling/vis/schemas/vis-config.schema.json`:
- Around line 1022-1055: The schema defines a scalar default for the object
property "firstSeen" (default: 0) which contradicts runtime behavior
(security.policies.firstSeen is left unset unless configured); remove the
"default": 0 entry from the "firstSeen" object schema in vis-config.schema.json
so the schema does not materialize a default policy and aligns with config.ts
handling of security.policies.firstSeen.
---
Nitpick comments:
In `@packages/tooling/vis/docs/commands/add.mdx`:
- Line 54: Update the `--run-scripts` description to remove the logical
ambiguity by explicitly stating its precedence relative to the block-by-default
policy and the allowlist: clarify whether `--run-scripts` globally enables
lifecycle scripts for the entire run (overriding the universal block-by-default
policy and causing scripts to run for all packages) or whether it only enables
scripts for non-allowlisted packages while the allowlist governed by
`security.policies.installScripts.allow` still controls allowlisted behavior;
edit the table row containing `--run-scripts` so it clearly states the chosen
semantics (e.g., "Enable lifecycle scripts for all packages in this run,
overriding the default block policy" OR "Enable scripts for packages that are
not already allowlisted; allowlisted packages still follow
security.policies.installScripts.allow") and pick one consistent phrasing.
In `@packages/tooling/vis/src/commands/hook/builtins/mixed-line-ending.ts`:
- Around line 29-33: The exported function's JSDoc is missing explicit types for
its parameters and return; update the JSDoc above the exported function that
accepts files, args, and context to include explicit types that match the
TypeScript signature (for example add `@param` {string[]} files, `@param` {string[]}
args, `@param` {YourContextType} context and an explicit `@returns` {number} or
{`@link` Promise<number>} if the function is async), ensuring the type name used
for context matches the actual type in the file.
In `@packages/tooling/vis/src/commands/hook/builtins/trailing-whitespace.ts`:
- Line 7: Replace the terse inline comment "// TAB, VT, FF, CR, SP" with a
purpose-focused comment that explains why those specific control characters are
treated as trailing-whitespace (e.g., "Treat horizontal tab, vertical tab, form
feed, carriage return and space as trailing whitespace so that CR from CRLF line
endings and other control whitespace are detected/trimmed") and mention the
rationale for including '\r' (to catch CR in CRLF pairs). Update the comment
adjacent to the trailing-whitespace logic in trailing-whitespace.ts to clearly
state intent rather than listing byte labels.
In `@packages/tooling/vis/src/report/audit-html.ts`:
- Around line 43-44: The manual HTML escaping in the escapeHtml function
currently handles &, <, >, " and ' but static analysis flagged it as potentially
error-prone; either (A) keep the implementation but add a clear comment above
escapeHtml explaining the local-only report context and why this minimal
escaping is sufficient (and add unit tests for edge cases like
null/empty/emoji/emoji combining sequences), or (B) replace the manual approach
by importing a vetted sanitizer (e.g. sanitize-html or DOMPurify), use its
safeEscape/escape function inside escapeHtml (preserve the escapeHtml symbol
name), and update tests and package.json accordingly so the codebase uses a
library for defense-in-depth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 65d91063-47ee-430a-952f-874608b93896
⛔ Files ignored due to path filters (13)
packages/tooling/vis/__tests__/commands/doctor/supply-chain.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/commands/init/init.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/config/config.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/schemas/schemas.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/config-writer.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/drift-check.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/policies/evaluate-integration.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/policies/install-scripts.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/policies/license.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/policies/unexpected-deps.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/policies/vulnerability.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/script-security.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/security.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**
📒 Files selected for processing (62)
packages/tooling/vis-mcp/src/tools/advisory-status.tspackages/tooling/vis-mcp/src/tools/audit.tspackages/tooling/vis/__bench__/audit-offline-fixture.tspackages/tooling/vis/docs/commands/add.mdxpackages/tooling/vis/docs/commands/approve-builds.mdxpackages/tooling/vis/docs/commands/audit.mdxpackages/tooling/vis/docs/commands/install.mdxpackages/tooling/vis/docs/commands/update.mdxpackages/tooling/vis/docs/guides/security-audit.mdxpackages/tooling/vis/native/src/advisories/osv.rspackages/tooling/vis/native/src/advisories_napi.rspackages/tooling/vis/package.jsonpackages/tooling/vis/rfc/design-offline-vuln-scanner.mdpackages/tooling/vis/schemas/vis-config.schema.jsonpackages/tooling/vis/src/commands/add/handler.tspackages/tooling/vis/src/commands/add/index.tspackages/tooling/vis/src/commands/advisories/index.tspackages/tooling/vis/src/commands/advisories/sync.tspackages/tooling/vis/src/commands/approve-builds/handler.tspackages/tooling/vis/src/commands/approve-builds/index.tspackages/tooling/vis/src/commands/audit/handler.tspackages/tooling/vis/src/commands/audit/index.tspackages/tooling/vis/src/commands/check/handler.tspackages/tooling/vis/src/commands/docker/handler.tspackages/tooling/vis/src/commands/doctor/filter.tspackages/tooling/vis/src/commands/doctor/handler.tspackages/tooling/vis/src/commands/doctor/supply-chain.tspackages/tooling/vis/src/commands/hook/builtins/check-merge-conflict.tspackages/tooling/vis/src/commands/hook/builtins/index.tspackages/tooling/vis/src/commands/hook/builtins/mixed-line-ending.tspackages/tooling/vis/src/commands/hook/builtins/trailing-whitespace.tspackages/tooling/vis/src/commands/hook/discover.tspackages/tooling/vis/src/commands/hook/dispatch.tspackages/tooling/vis/src/commands/hook/prek.tspackages/tooling/vis/src/commands/hook/run.tspackages/tooling/vis/src/commands/init/handler.tspackages/tooling/vis/src/commands/install/handler.tspackages/tooling/vis/src/commands/install/index.tspackages/tooling/vis/src/commands/security/index.tspackages/tooling/vis/src/commands/security/list.tspackages/tooling/vis/src/commands/security/run.tspackages/tooling/vis/src/commands/security/sync.tspackages/tooling/vis/src/commands/update/handler.tspackages/tooling/vis/src/config/config-writer.tspackages/tooling/vis/src/config/config.tspackages/tooling/vis/src/config/types.tspackages/tooling/vis/src/plugins/security-enforcement.tspackages/tooling/vis/src/report/audit-html.tspackages/tooling/vis/src/report/sarif.tspackages/tooling/vis/src/security/config-check.tspackages/tooling/vis/src/security/drift.tspackages/tooling/vis/src/security/enforcement.tspackages/tooling/vis/src/security/manifests.tspackages/tooling/vis/src/security/min-release-age.tspackages/tooling/vis/src/security/native-config-sync.tspackages/tooling/vis/src/security/policies/index.tspackages/tooling/vis/src/security/policies/install-scripts.tspackages/tooling/vis/src/security/policies/license.tspackages/tooling/vis/src/security/policies/unexpected-deps.tspackages/tooling/vis/src/security/policies/vulnerability.tspackages/tooling/vis/src/security/socket-security.tspackages/tooling/vis/src/util/catalog.ts
✅ Files skipped from review due to trivial changes (13)
- packages/tooling/vis/src/commands/hook/builtins/index.ts
- packages/tooling/vis/src/commands/hook/run.ts
- packages/tooling/vis/src/commands/hook/dispatch.ts
- packages/tooling/vis/src/commands/docker/handler.ts
- packages/tooling/vis/src/commands/hook/prek.ts
- packages/tooling/vis/src/commands/hook/discover.ts
- packages/tooling/vis/src/commands/approve-builds/index.ts
- packages/tooling/vis/src/commands/add/index.ts
- packages/tooling/vis/src/commands/hook/builtins/check-merge-conflict.ts
- packages/tooling/vis/docs/commands/update.mdx
- packages/tooling/vis/src/commands/install/index.ts
- packages/tooling/vis/docs/commands/install.mdx
- packages/tooling/vis/docs/commands/approve-builds.mdx
🚧 Files skipped from review as they are similar to previous changes (22)
- packages/tooling/vis/src/commands/security/index.ts
- packages/tooling/vis/src/plugins/security-enforcement.ts
- packages/tooling/vis/src/commands/security/list.ts
- packages/tooling/vis/src/commands/security/run.ts
- packages/tooling/vis/src/commands/install/handler.ts
- packages/tooling/vis/src/commands/security/sync.ts
- packages/tooling/vis/src/commands/approve-builds/handler.ts
- packages/tooling/vis/src/commands/check/handler.ts
- packages/tooling/vis/package.json
- packages/tooling/vis-mcp/src/tools/audit.ts
- packages/tooling/vis/src/commands/doctor/supply-chain.ts
- packages/tooling/vis/src/commands/add/handler.ts
- packages/tooling/vis/native/src/advisories_napi.rs
- packages/tooling/vis/src/config/config-writer.ts
- packages/tooling/vis/src/commands/advisories/index.ts
- packages/tooling/vis/src/commands/advisories/sync.ts
- packages/tooling/vis/src/commands/doctor/handler.ts
- packages/tooling/vis/bench/audit-offline-fixture.ts
- packages/tooling/vis/native/src/advisories/osv.rs
- packages/tooling/vis/src/commands/audit/handler.ts
- packages/tooling/vis/src/commands/init/handler.ts
- packages/tooling/vis/src/config/types.ts
Comment on lines
+151
to
+292
| export const emitSarif = (options: SarifEmitOptions): SarifLog => { | ||
| const rulesById = new Map<string, SarifRule>(); | ||
| const results: SarifResult[] = []; | ||
| const lockfileUri = options.artifactUri ?? (relative(options.workspaceRoot, join(options.workspaceRoot, "package.json")) || "package.json"); | ||
|
|
||
| for (const finding of options.findings) { | ||
| const { acknowledged, packageName, packageVersion, vulnerability: vuln } = finding; | ||
| const level = SEVERITY_TO_LEVEL[vuln.severity] ?? "none"; | ||
| const label = SEVERITY_TO_LABEL[vuln.severity] ?? "none"; | ||
|
|
||
| if (!rulesById.has(vuln.id)) { | ||
| rulesById.set(vuln.id, { | ||
| defaultConfiguration: { level }, | ||
| fullDescription: { text: vuln.summary || `Advisory ${vuln.id}` }, | ||
| helpUri: advisoryUri(vuln.id), | ||
| id: vuln.id, | ||
| name: vuln.id, | ||
| properties: { | ||
| precision: "very-high", | ||
| "security-severity": securitySeverity(vuln), | ||
| "severity-label": label, | ||
| tags: ["security", "vulnerability", "supply-chain", `severity:${label}`], | ||
| }, | ||
| shortDescription: { text: (vuln.summary.split("\n")[0] ?? vuln.id).slice(0, 200) }, | ||
| }); | ||
| } | ||
|
|
||
| results.push({ | ||
| level, | ||
| locations: [ | ||
| { | ||
| logicalLocations: [{ kind: "package", name: `${packageName}@${packageVersion}` }], | ||
| physicalLocation: { | ||
| artifactLocation: { uri: lockfileUri }, | ||
| }, | ||
| }, | ||
| ], | ||
| message: { | ||
| text: `${vuln.id}: ${packageName}@${packageVersion} — ${vuln.summary || "no summary"}${ | ||
| vuln.fixedVersions.length > 0 ? ` (fix: ${vuln.fixedVersions.join(", ")})` : "" | ||
| }`, | ||
| }, | ||
| partialFingerprints: { | ||
| advisoryId: vuln.id, | ||
| package: packageName, | ||
| version: packageVersion, | ||
| }, | ||
| properties: { | ||
| ...(acknowledged ? { acknowledged: true } : {}), | ||
| ...(vuln.aliases && vuln.aliases.length > 0 ? { aliases: vuln.aliases } : {}), | ||
| ...(typeof vuln.cvssScore === "number" ? { cvssScore: vuln.cvssScore } : {}), | ||
| ...(vuln.fixedVersions.length > 0 ? { fixedVersions: vuln.fixedVersions } : {}), | ||
| packageName, | ||
| packageVersion, | ||
| severityLabel: label, | ||
| }, | ||
| ruleId: vuln.id, | ||
| }); | ||
| } | ||
|
|
||
| // Non-vulnerability policy decisions ride alongside advisories. One | ||
| // SARIF rule per policy name keeps the rule table small while still | ||
| // letting Code Scanning render the right severity badge per row. | ||
| const policyLevel: Record<PolicyDecision["severity"], SarifLevel> = { | ||
| block: "error", | ||
| info: "note", | ||
| warn: "warning", | ||
| }; | ||
| const policyLabel: Record<PolicyDecision["severity"], string> = { | ||
| block: "high", | ||
| info: "none", | ||
| warn: "medium", | ||
| }; | ||
|
|
||
| for (const decision of options.policyDecisions ?? []) { | ||
| if (decision.policy === "vulnerability") { | ||
| // Already covered by the OSV pass above. | ||
| continue; | ||
| } | ||
|
|
||
| const ruleId = `vis.policy.${decision.policy}`; | ||
| const level = policyLevel[decision.severity]; | ||
| const label = policyLabel[decision.severity]; | ||
|
|
||
| if (!rulesById.has(ruleId)) { | ||
| rulesById.set(ruleId, { | ||
| defaultConfiguration: { level }, | ||
| fullDescription: { text: `vis policy '${decision.policy}' (Socket.dev-style supply-chain gate)` }, | ||
| id: ruleId, | ||
| name: ruleId, | ||
| properties: { | ||
| precision: "high", | ||
| "security-severity": decision.severity === "block" ? "8.0" : decision.severity === "warn" ? "5.5" : "0.0", | ||
| "severity-label": label, | ||
| tags: ["security", "supply-chain", "policy", `policy:${decision.policy}`], | ||
| }, | ||
| shortDescription: { text: `vis policy: ${decision.policy}` }, | ||
| }); | ||
| } | ||
|
|
||
| results.push({ | ||
| level, | ||
| locations: [ | ||
| { | ||
| logicalLocations: [{ kind: "package", name: `${decision.packageName}@${decision.version}` }], | ||
| physicalLocation: { artifactLocation: { uri: lockfileUri } }, | ||
| }, | ||
| ], | ||
| message: { text: decision.reason }, | ||
| partialFingerprints: { | ||
| package: decision.packageName, | ||
| policy: decision.policy, | ||
| version: decision.version, | ||
| }, | ||
| properties: { | ||
| ...(decision.acceptedRisk ? { acknowledged: true } : {}), | ||
| packageName: decision.packageName, | ||
| packageVersion: decision.version, | ||
| severityLabel: label, | ||
| }, | ||
| ruleId, | ||
| }); | ||
| } | ||
|
|
||
| return { | ||
| $schema: "https://json.schemastore.org/sarif-2.1.0.json", | ||
| runs: [ | ||
| { | ||
| results, | ||
| tool: { | ||
| driver: { | ||
| informationUri: options.tool.informationUri, | ||
| name: options.tool.name, | ||
| rules: [...rulesById.values()], | ||
| version: options.tool.version, | ||
| }, | ||
| }, | ||
| }, | ||
| ], | ||
| version: "2.1.0", | ||
| }; | ||
| }; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc comment for the exported function.
The emitSarif function is exported but lacks a JSDoc comment. As per coding guidelines, exported functions should include JSDoc with parameter types and return types.
📝 Suggested JSDoc
+/**
+ * Emits a SARIF 2.1.0 log for `vis audit` findings.
+ *
+ * Generates one run with rules keyed by advisory/policy ID and results
+ * for each (package, advisory) pair. Includes severity mapping, CVSS scores,
+ * partial fingerprints for deduplication, and policy decisions.
+ *
+ * `@param` options - Configuration including findings, policy decisions, and tool metadata
+ * `@returns` SARIF 2.1.0 log object conforming to the JSON schema
+ */
export const emitSarif = (options: SarifEmitOptions): SarifLog => {As per coding guidelines: "Always include JSDoc comments for exported functions with parameter types and return types".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const emitSarif = (options: SarifEmitOptions): SarifLog => { | |
| const rulesById = new Map<string, SarifRule>(); | |
| const results: SarifResult[] = []; | |
| const lockfileUri = options.artifactUri ?? (relative(options.workspaceRoot, join(options.workspaceRoot, "package.json")) || "package.json"); | |
| for (const finding of options.findings) { | |
| const { acknowledged, packageName, packageVersion, vulnerability: vuln } = finding; | |
| const level = SEVERITY_TO_LEVEL[vuln.severity] ?? "none"; | |
| const label = SEVERITY_TO_LABEL[vuln.severity] ?? "none"; | |
| if (!rulesById.has(vuln.id)) { | |
| rulesById.set(vuln.id, { | |
| defaultConfiguration: { level }, | |
| fullDescription: { text: vuln.summary || `Advisory ${vuln.id}` }, | |
| helpUri: advisoryUri(vuln.id), | |
| id: vuln.id, | |
| name: vuln.id, | |
| properties: { | |
| precision: "very-high", | |
| "security-severity": securitySeverity(vuln), | |
| "severity-label": label, | |
| tags: ["security", "vulnerability", "supply-chain", `severity:${label}`], | |
| }, | |
| shortDescription: { text: (vuln.summary.split("\n")[0] ?? vuln.id).slice(0, 200) }, | |
| }); | |
| } | |
| results.push({ | |
| level, | |
| locations: [ | |
| { | |
| logicalLocations: [{ kind: "package", name: `${packageName}@${packageVersion}` }], | |
| physicalLocation: { | |
| artifactLocation: { uri: lockfileUri }, | |
| }, | |
| }, | |
| ], | |
| message: { | |
| text: `${vuln.id}: ${packageName}@${packageVersion} — ${vuln.summary || "no summary"}${ | |
| vuln.fixedVersions.length > 0 ? ` (fix: ${vuln.fixedVersions.join(", ")})` : "" | |
| }`, | |
| }, | |
| partialFingerprints: { | |
| advisoryId: vuln.id, | |
| package: packageName, | |
| version: packageVersion, | |
| }, | |
| properties: { | |
| ...(acknowledged ? { acknowledged: true } : {}), | |
| ...(vuln.aliases && vuln.aliases.length > 0 ? { aliases: vuln.aliases } : {}), | |
| ...(typeof vuln.cvssScore === "number" ? { cvssScore: vuln.cvssScore } : {}), | |
| ...(vuln.fixedVersions.length > 0 ? { fixedVersions: vuln.fixedVersions } : {}), | |
| packageName, | |
| packageVersion, | |
| severityLabel: label, | |
| }, | |
| ruleId: vuln.id, | |
| }); | |
| } | |
| // Non-vulnerability policy decisions ride alongside advisories. One | |
| // SARIF rule per policy name keeps the rule table small while still | |
| // letting Code Scanning render the right severity badge per row. | |
| const policyLevel: Record<PolicyDecision["severity"], SarifLevel> = { | |
| block: "error", | |
| info: "note", | |
| warn: "warning", | |
| }; | |
| const policyLabel: Record<PolicyDecision["severity"], string> = { | |
| block: "high", | |
| info: "none", | |
| warn: "medium", | |
| }; | |
| for (const decision of options.policyDecisions ?? []) { | |
| if (decision.policy === "vulnerability") { | |
| // Already covered by the OSV pass above. | |
| continue; | |
| } | |
| const ruleId = `vis.policy.${decision.policy}`; | |
| const level = policyLevel[decision.severity]; | |
| const label = policyLabel[decision.severity]; | |
| if (!rulesById.has(ruleId)) { | |
| rulesById.set(ruleId, { | |
| defaultConfiguration: { level }, | |
| fullDescription: { text: `vis policy '${decision.policy}' (Socket.dev-style supply-chain gate)` }, | |
| id: ruleId, | |
| name: ruleId, | |
| properties: { | |
| precision: "high", | |
| "security-severity": decision.severity === "block" ? "8.0" : decision.severity === "warn" ? "5.5" : "0.0", | |
| "severity-label": label, | |
| tags: ["security", "supply-chain", "policy", `policy:${decision.policy}`], | |
| }, | |
| shortDescription: { text: `vis policy: ${decision.policy}` }, | |
| }); | |
| } | |
| results.push({ | |
| level, | |
| locations: [ | |
| { | |
| logicalLocations: [{ kind: "package", name: `${decision.packageName}@${decision.version}` }], | |
| physicalLocation: { artifactLocation: { uri: lockfileUri } }, | |
| }, | |
| ], | |
| message: { text: decision.reason }, | |
| partialFingerprints: { | |
| package: decision.packageName, | |
| policy: decision.policy, | |
| version: decision.version, | |
| }, | |
| properties: { | |
| ...(decision.acceptedRisk ? { acknowledged: true } : {}), | |
| packageName: decision.packageName, | |
| packageVersion: decision.version, | |
| severityLabel: label, | |
| }, | |
| ruleId, | |
| }); | |
| } | |
| return { | |
| $schema: "https://json.schemastore.org/sarif-2.1.0.json", | |
| runs: [ | |
| { | |
| results, | |
| tool: { | |
| driver: { | |
| informationUri: options.tool.informationUri, | |
| name: options.tool.name, | |
| rules: [...rulesById.values()], | |
| version: options.tool.version, | |
| }, | |
| }, | |
| }, | |
| ], | |
| version: "2.1.0", | |
| }; | |
| }; | |
| /** | |
| * Emits a SARIF 2.1.0 log for `vis audit` findings. | |
| * | |
| * Generates one run with rules keyed by advisory/policy ID and results | |
| * for each (package, advisory) pair. Includes severity mapping, CVSS scores, | |
| * partial fingerprints for deduplication, and policy decisions. | |
| * | |
| * `@param` options - Configuration including findings, policy decisions, and tool metadata | |
| * `@returns` SARIF 2.1.0 log object conforming to the JSON schema | |
| */ | |
| export const emitSarif = (options: SarifEmitOptions): SarifLog => { | |
| const rulesById = new Map<string, SarifRule>(); | |
| const results: SarifResult[] = []; | |
| const lockfileUri = options.artifactUri ?? (relative(options.workspaceRoot, join(options.workspaceRoot, "package.json")) || "package.json"); | |
| for (const finding of options.findings) { | |
| const { acknowledged, packageName, packageVersion, vulnerability: vuln } = finding; | |
| const level = SEVERITY_TO_LEVEL[vuln.severity] ?? "none"; | |
| const label = SEVERITY_TO_LABEL[vuln.severity] ?? "none"; | |
| if (!rulesById.has(vuln.id)) { | |
| rulesById.set(vuln.id, { | |
| defaultConfiguration: { level }, | |
| fullDescription: { text: vuln.summary || `Advisory ${vuln.id}` }, | |
| helpUri: advisoryUri(vuln.id), | |
| id: vuln.id, | |
| name: vuln.id, | |
| properties: { | |
| precision: "very-high", | |
| "security-severity": securitySeverity(vuln), | |
| "severity-label": label, | |
| tags: ["security", "vulnerability", "supply-chain", `severity:${label}`], | |
| }, | |
| shortDescription: { text: (vuln.summary.split("\n")[0] ?? vuln.id).slice(0, 200) }, | |
| }); | |
| } | |
| results.push({ | |
| level, | |
| locations: [ | |
| { | |
| logicalLocations: [{ kind: "package", name: `${packageName}@${packageVersion}` }], | |
| physicalLocation: { | |
| artifactLocation: { uri: lockfileUri }, | |
| }, | |
| }, | |
| ], | |
| message: { | |
| text: `${vuln.id}: ${packageName}@${packageVersion} — ${vuln.summary || "no summary"}${ | |
| vuln.fixedVersions.length > 0 ? ` (fix: ${vuln.fixedVersions.join(", ")})` : "" | |
| }`, | |
| }, | |
| partialFingerprints: { | |
| advisoryId: vuln.id, | |
| package: packageName, | |
| version: packageVersion, | |
| }, | |
| properties: { | |
| ...(acknowledged ? { acknowledged: true } : {}), | |
| ...(vuln.aliases && vuln.aliases.length > 0 ? { aliases: vuln.aliases } : {}), | |
| ...(typeof vuln.cvssScore === "number" ? { cvssScore: vuln.cvssScore } : {}), | |
| ...(vuln.fixedVersions.length > 0 ? { fixedVersions: vuln.fixedVersions } : {}), | |
| packageName, | |
| packageVersion, | |
| severityLabel: label, | |
| }, | |
| ruleId: vuln.id, | |
| }); | |
| } | |
| // Non-vulnerability policy decisions ride alongside advisories. One | |
| // SARIF rule per policy name keeps the rule table small while still | |
| // letting Code Scanning render the right severity badge per row. | |
| const policyLevel: Record<PolicyDecision["severity"], SarifLevel> = { | |
| block: "error", | |
| info: "note", | |
| warn: "warning", | |
| }; | |
| const policyLabel: Record<PolicyDecision["severity"], string> = { | |
| block: "high", | |
| info: "none", | |
| warn: "medium", | |
| }; | |
| for (const decision of options.policyDecisions ?? []) { | |
| if (decision.policy === "vulnerability") { | |
| // Already covered by the OSV pass above. | |
| continue; | |
| } | |
| const ruleId = `vis.policy.${decision.policy}`; | |
| const level = policyLevel[decision.severity]; | |
| const label = policyLabel[decision.severity]; | |
| if (!rulesById.has(ruleId)) { | |
| rulesById.set(ruleId, { | |
| defaultConfiguration: { level }, | |
| fullDescription: { text: `vis policy '${decision.policy}' (Socket.dev-style supply-chain gate)` }, | |
| id: ruleId, | |
| name: ruleId, | |
| properties: { | |
| precision: "high", | |
| "security-severity": decision.severity === "block" ? "8.0" : decision.severity === "warn" ? "5.5" : "0.0", | |
| "severity-label": label, | |
| tags: ["security", "supply-chain", "policy", `policy:${decision.policy}`], | |
| }, | |
| shortDescription: { text: `vis policy: ${decision.policy}` }, | |
| }); | |
| } | |
| results.push({ | |
| level, | |
| locations: [ | |
| { | |
| logicalLocations: [{ kind: "package", name: `${decision.packageName}@${decision.version}` }], | |
| physicalLocation: { artifactLocation: { uri: lockfileUri } }, | |
| }, | |
| ], | |
| message: { text: decision.reason }, | |
| partialFingerprints: { | |
| package: decision.packageName, | |
| policy: decision.policy, | |
| version: decision.version, | |
| }, | |
| properties: { | |
| ...(decision.acceptedRisk ? { acknowledged: true } : {}), | |
| packageName: decision.packageName, | |
| packageVersion: decision.version, | |
| severityLabel: label, | |
| }, | |
| ruleId, | |
| }); | |
| } | |
| return { | |
| $schema: "https://json.schemastore.org/sarif-2.1.0.json", | |
| runs: [ | |
| { | |
| results, | |
| tool: { | |
| driver: { | |
| informationUri: options.tool.informationUri, | |
| name: options.tool.name, | |
| rules: [...rulesById.values()], | |
| version: options.tool.version, | |
| }, | |
| }, | |
| }, | |
| ], | |
| version: "2.1.0", | |
| }; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tooling/vis/src/report/sarif.ts` around lines 151 - 292, The
exported function emitSarif lacks a JSDoc block: add a JSDoc comment immediately
above the export const emitSarif declaration that documents the parameter
(options: SarifEmitOptions) and the return type (SarifLog), briefly describes
the function purpose, and notes important option fields used (e.g., findings,
policyDecisions, workspaceRoot, artifactUri, tool) so IDEs and docs pick up
types and intent; ensure the JSDoc includes `@param` {SarifEmitOptions} options
and `@returns` {SarifLog}.
npm CLI types `min-release-age` as `Number` in days, not a duration string — vis wrote `48h`/`15m`, which npm's parseInt would silently read as 48/15 days. Write integer days rounded up so the native gate is never weaker than vis-config. Yarn Berry silently treats day suffixes in `npmMinimalAgeGate` as minutes (yarnpkg/berry#6991). Write a bare integer minute count to dodge the bug, and add support for `npmPreapprovedPackages` (yarn's native exclude list). Update drift + update-handler readers to interpret `.npmrc` bare integers as days while still accepting legacy `Nd`/`Nh`/`Nm` strings for back-compat. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Round-3 fixes that complete the offline-clean policy engine review: - SPDX leaf extraction now correctly handles `WITH` exceptions, `<id>+` or-later modifiers, and `LicenseRef-*` refs (license policy) - forward `security.pinVersions` into `scanBuildScriptStatus` so the install-scripts policy respects version-pinned allow entries - SARIF policy rules get `helpUri` pointing at the audit docs anchor - `readNodeModulesManifests` skips symlinks (lstat-based) and guards against cycles via a visited set, preventing infinite recursion when `node_modules` contains workspace-linked packages - only invoke the manifest walk when the license policy is both configured and enabled (`--policies` filter aware) - single source of truth for `POLICY_NAMES` in `config/types.ts`; `parsePoliciesFlag` accepts case-insensitive, snake_case, and leading-underscore spellings - share `SEVERITY_ORDER` / `severityPassesFilter` between the audit handler and the vulnerability policy via a new `security/severity.ts` - share `LOCKFILE_NAMES` between `dependency-scan` and the `unexpected-deps` baseline filename detector Adds test coverage for SPDX edge cases, expired and policy-scoped accepted-risk handling, vulnerability with empty findings, SARIF policy emission with `helpUri`, HTML Policy Decisions section, manifest symlink and cycle protection, and `parsePoliciesFlag` case-insensitive / underscore-stripping behaviour. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/tooling/vis/src/commands/update/handler.ts (1)
108-115: ⚡ Quick winAdd typed JSDoc tags to the exported helper.
This new exported function has a descriptive block comment, but it still omits the typed
@paramand@returnstags the repo requires for exported TS helpers.Suggested docblock update
/** * Parses a `.npmrc` `min-release-age` value into minutes. npm's CLI defines * this option as `null or Number` measured in **days**, so a bare integer * like `"1"` means one day — not one minute. Legacy `Nd`/`Nh`/`Nm` strings * (older vis writes and hand-edits) still fall back to `parseTimeStringToMinutes` * for forgiving drift comparison until `vis security sync` rewrites them. + * + * `@param` {string} raw Raw `min-release-age` value from `.npmrc`. + * `@returns` {number | undefined} Normalized minutes, or `undefined` for malformed input. */ export const parseNpmReleaseAgeValue = (raw: string): number | undefined => {As per coding guidelines, "Always include JSDoc comments for exported functions with parameter types and return types".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tooling/vis/src/commands/update/handler.ts` around lines 108 - 115, The exported helper parseNpmReleaseAgeValue is missing typed JSDoc tags; add a JSDoc block that includes an `@param` {string} raw description describing accepted formats (e.g., bare integer days, Nd/Nh/Nm legacy strings) and an `@returns` {(number|undefined)} description stating it returns minutes or undefined on invalid input, and ensure the overall summary stays intact above the function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/tooling/vis/src/commands/update/handler.ts`:
- Around line 118-122: The numeric-only regex rejects decimals so strings like
"1.5" are treated as time strings instead of days; update the conditional to
accept decimal numbers (use /^\d+(\.\d+)?$/) and convert with
Number.parseFloat(trimmed) * 1440 to produce minutes, leaving
parseTimeStringToMinutes(trimmed) for non-numeric inputs; also add JSDoc
annotations to the exported function that contains this snippet (the exported
function that returns minutes and calls parseTimeStringToMinutes) documenting
the parameter type (string) and return type (number).
---
Nitpick comments:
In `@packages/tooling/vis/src/commands/update/handler.ts`:
- Around line 108-115: The exported helper parseNpmReleaseAgeValue is missing
typed JSDoc tags; add a JSDoc block that includes an `@param` {string} raw
description describing accepted formats (e.g., bare integer days, Nd/Nh/Nm
legacy strings) and an `@returns` {(number|undefined)} description stating it
returns minutes or undefined on invalid input, and ensure the overall summary
stays intact above the function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 36362257-14b9-46e5-b42f-09eec4e4fe69
⛔ Files ignored due to path filters (10)
packages/tooling/vis/__tests__/commands/update/native-min-release-age.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/report/audit-html.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/report/sarif.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/drift-check.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/manifests.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/min-release-age-sync.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/policies/evaluate-integration.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/policies/install-scripts.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/policies/license.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/tooling/vis/__tests__/security/policies/vulnerability.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**
📒 Files selected for processing (14)
packages/tooling/vis/src/commands/audit/handler.tspackages/tooling/vis/src/commands/update/handler.tspackages/tooling/vis/src/config/types.tspackages/tooling/vis/src/report/sarif.tspackages/tooling/vis/src/security/dependency-scan.tspackages/tooling/vis/src/security/drift.tspackages/tooling/vis/src/security/manifests.tspackages/tooling/vis/src/security/min-release-age.tspackages/tooling/vis/src/security/policies/index.tspackages/tooling/vis/src/security/policies/install-scripts.tspackages/tooling/vis/src/security/policies/license.tspackages/tooling/vis/src/security/policies/unexpected-deps.tspackages/tooling/vis/src/security/policies/vulnerability.tspackages/tooling/vis/src/security/severity.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/tooling/vis/src/report/sarif.ts
- packages/tooling/vis/src/commands/audit/handler.ts
- packages/tooling/vis/src/config/types.ts
…ngine Deep-merge security.policies sub-bodies across the extends chain so a preset that sets installScripts.allow isn't wiped when the consumer config sets installScripts.strict. Mirrors mergeSecurityDefaults. Two regression tests added. npm min-release-age values like "1.5" (decimal days) now parse correctly — npm's option type is Number measured in days, so bare-numeric reads multiply by 1440 rather than falling through to parseTimeStringToMinutes. Schema tweaks: drop the misleading "default: 0" on firstSeen, clarify in the policies description that installScripts.strict and publisherChange.mode remain on via SECURITY_DEFAULTS. Small additions: helpUri on policy SARIF rules, one-line JSDoc on emitSarif and emitAuditHtml, tightened zod schema on vis-mcp advisory_status (non-empty optional string + explicit emptiness check), and clarified add.mdx wording on the --run-scripts one-off opt-out. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds 8 npq-parity marshalls (author, provenance, newBin, metadata, downloads, expiredDomains, signatures, archivedRepo) that inspect packages before vis add. The pipeline runs marshalls in parallel with bounded concurrency, prefetches packuments once globally, and dedupes shared DNS/GitHub lookups across packages so a monorepo adding 50 packages from 3 maintainers makes 3 DNS calls, not 50. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Runs the offline marshall pipeline before vis add and before vis update on the explicit-args branch (blanket update skips it to avoid a packument fan-out over every installed dep). Adds --no-marshall-check escape hatches on both commands. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Lets users run the marshall pipeline against arbitrary package specs without installing — useful for vetting a dep before adding it, or for CI checks against a list. Reuses the same config gating and decision-prompt rendering as vis add. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…gnostics BREAKING: rename four VisConfig keys to plain-English names: targetDefaults -> tasks taskDefaults -> scopedTasks (with .scope -> .match, .targets -> .tasks) taskRunnerOptions -> taskRunner VisTaskConfig.targets -> tasks A new deprecation pass runs during config load and throws VisConfigDeprecatedKeyError on the old names with a per-key suggested replacement, so the upgrade path is a single error message instead of silent drift. Migrate adapters (moon, nx, turborepo), schema, docs, and guides are all updated to the new names; new guides/config-model and guides/shell-alias cover the mental model. Hook command groups are retitled "Git Hooks" while we're here. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`vis migrate self` rewrites vis.config.ts / vis.task.ts to use the current field names after a hard-break rename (paired with the deprecation diagnostics so the upgrade path is: see error -> run migrate self). A .bak is written next to each modified file; --dry-run previews. `vis migrate all` probes the workspace via a new registry-based applicability check and runs every detected migration in one go, so users upgrading from a mixed toolchain (turborepo + secretlint + syncpack) don't have to remember each subcommand. Migrate-related commands are regrouped under "Migrate" in --help. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Planning document covering 12 npq-parity marshalls for vis, of which 8 are now landed (author, provenance, newBin, metadata, downloads, expiredDomains, signatures, archivedRepo). Kept in the repo so the remaining four (resolve-explicit, registry-keys, verify-ecdsa, decision-prompt presets) have visible scope. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
prisis
pushed a commit
that referenced
this pull request
May 14, 2026
…a/vis [1.0.0-alpha.20](https://github.com/visulima/visulima/compare/@visulima/vis@1.0.0-alpha.19...@visulima/vis@1.0.0-alpha.20) (2026-05-14) ### ⚠ BREAKING CHANGES * **vis:** the following security.* keys were renamed: - security.minimumReleaseAge -> security.policies.first_seen.minutes - security.minimumReleaseAgeExclude -> security.policies.first_seen.exclude - security.trustPolicy -> security.policies.publisher_change.mode - security.trustPolicyExclude -> security.policies.publisher_change.exclude - security.trustPolicyIgnoreAfter -> security.policies.publisher_change.ignoreAfter - security.allowBuilds -> security.policies.install_scripts.allow - security.strictDepBuilds -> security.policies.install_scripts.strict - security.socket.minimumScore -> security.policies.score.minimum - security.socket.acceptedRisks -> security.acceptedRisks - security.audit.failOn -> security.policies.vulnerability.failOn - security.audit.usage -> security.policies.vulnerability.usage AcceptedRisk now carries optional policies[] and expiresAt fields so risks can be scoped per-policy and time-boxed. Native PM sync writers keep emitting pnpm-native field names since pnpm owns that schema. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(vis): address coderabbit review feedback - Add JSDoc to exported items across hook builtins, advisories, audit, docker - Document OSV `last_affected` inclusive-upper semantics with a focused test - Annotate NAPI u64→u32 truncations with JS Number range rationale - Switch vis-mcp tool payloads to zod schemas with `.catchall(z.unknown())` so unknown CLI fields stay forward-compatible - Fence RFC code blocks with explicit languages and tidy hook-command formatting (multi-line type, implicit-return arrows) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(vis): address second-round coderabbit feedback on policies rename - Drift report labels now reference `security.policies.*` paths so the hint matches the keys users edit - `defineConfig` JSDoc examples use the new `security.policies` shape - `mergeSecurityDefaults` deep-merges every defaulted sub-policy generically; `mergeVisConfigs` deep-merges `policies` and `acceptedRisks` so presets aren't clobbered - Config-writer scopes its `allow:` match to follow an `install_scripts:` opener - JSDoc clarifications for `audit.advisories.source` default, `acceptedRisks.expiresAt`/`acceptedScore` ranges, `malware.mode` cross-field default, and the current `policies.score.minimum` wiring gap Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(vis): finish coderabbit round-2 fixes — camelCase policies + wire score.minimum - Rename `PolicyName` union to camelCase (`firstSeen`, `installScripts`, `publisherChange`, `unexpectedDeps`). Updates every consumer in src/, test fixtures, schemas, and docs. JSON schema regenerated. - Add `types` mapping to the `#native` package.json import so TypeScript resolves `index.d.ts` when consumers import the alias. - Thread `socketOptions.minimumScore` through `audit`, `doctor`, `check`, `update`, `add`, `formatSecurityOverview`, `formatSummary`, and `applyFilter`. `buildSocketOptions` now resolves the effective minimum once (from `security.policies.score.minimum` or `DEFAULT_LOW_SCORE_THRESHOLD`) so every consumer sees the same value instead of comparing scores against the hard-coded constant. - Update the JSDoc on `policies.score.minimum` to reflect the new wiring. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(vis): add Socket-style policy engine with 4 offline-clean policies Introduce a unified `evaluatePolicies()` engine under `src/security/policies/` with four offline-clean modules: license, install_scripts, vulnerability, and unexpected_deps (baseline mode). Each policy emits PolicyDecisions (block/warn/info) keyed by package, with per-policy accepted-risk scoping and expiresAt support reused from the shared matcher. Wires the engine into `vis audit`: a new `--policies <names>` flag (comma-list, `all`, or `none`) narrows evaluation. Block-severity decisions feed into `--exit-code` and `--fail-on`. JSON output gains `policies[]` + `summary.policyBlocks`. SARIF and HTML formatters render policy decisions alongside vulnerabilities; CSAF and CycloneDX-VEX are intentionally left untouched (vuln-specific data models). Adds `readNodeModulesManifests()` to walk `node_modules/` (including the pnpm `.pnpm/` content-addressed store) and surface license + scripts + maintainers metadata for the offline policies. 33 new unit + integration tests; full vitest suite (3582/3582) green; `tsc --noEmit` clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(vis): close three audit policy gating gaps surfaced by CodeRabbit - applyExitGate's fallthrough call to applyFailOnGate was dropping the policyDecisions argument, so `vis audit --format sarif --fail-on high` exited 0 when the only signal was a block-severity policy decision. Forward the argument. - Unknown --policies tokens were silently swallowed when format was json/sarif/csaf/cyclonedx-vex, so a typoed CI invocation reduced enforcement with no log. Always emit the warning to stderr and surface the tokens in JSON output as `warnings[]`. - Vulnerability-policy block decisions whose advisory was masked by --severity used to exit 1 with no visible reason. Surface those decisions in the human-readable "Policy Decisions" section so the gate is always traceable. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(vis): correct npm/yarn min-release-age native config writers npm CLI types `min-release-age` as `Number` in days, not a duration string — vis wrote `48h`/`15m`, which npm's parseInt would silently read as 48/15 days. Write integer days rounded up so the native gate is never weaker than vis-config. Yarn Berry silently treats day suffixes in `npmMinimalAgeGate` as minutes ### Features * **vis:** add LavaMoat allow-scripts parity (run/tripwire/--write/allowBins) ([84218d3](84218d3)) * **vis:** multi-source codeowners aggregation ([d22df81](d22df81)) * **vis:** offline OSV scanner + unified security.policies ([#632](#632)) ([6461902](6461902)), closes [#631](#631) [#631](#631) [yarnpkg/berry#6991](yarnpkg/berry#6991) * **vis:** wire marshall env-var matrix into install/audit/check + add keys-refresh ([e1e2d6c](e1e2d6c)) ### Bug Fixes * **release:** patch NAPI version-check string and ship fresh loader on release ([0676e33](0676e33)) * **vis:** clear lint findings in hook dispatch, builtins, and util ([d05204c](d05204c)) ### Documentation * **vis:** add vltpkg/security-archive attribution ([019d6fd](019d6fd)) ### Miscellaneous Chores * fixed build ([ec156bf](ec156bf)) * update license file ([8a84e10](8a84e10)) * **vis:** apply prettier and eslint --fix sweep ([ec64552](ec64552)) * **vis:** clear remaining ESLint findings across marshalls and tests ([29f87c5](29f87c5)) * **vis:** fix indent-binary-ops and silence default-log no-console ([9c8d5e1](9c8d5e1)) * **vis:** style normalization sweep + scopedTasks/allowBins config fields ([ff97758](ff97758)) ### Tests * **vis:** raise audit-offline gate to 5× budget for CI hosts ([345b159](345b159)) * **vis:** use median-of-11 samples for audit-offline perf gate ([3225515](3225515)) ### Continuous Integration * **vis:** track index.d.ts so loader artifact survives cache hits ([b9a439f](b9a439f)) ### Dependencies * **@visulima/secret-scanner:** upgraded to 1.0.0-alpha.3 * **@visulima/task-runner:** upgraded to 1.0.0-alpha.14 * **@visulima/tui:** upgraded to 1.0.0-alpha.15 * **@visulima/cerebro:** upgraded to 3.0.0-alpha.23 * **@visulima/colorize:** upgraded to 2.0.0-alpha.11 * **@visulima/fs:** upgraded to 5.0.0-alpha.22 * **@visulima/package:** upgraded to 5.0.0-alpha.21 * **@visulima/pail:** upgraded to 4.0.0-alpha.16
prisis
pushed a commit
that referenced
this pull request
May 14, 2026
…ulima/vis-mcp [1.0.0-alpha.10](https://github.com/visulima/visulima/compare/@visulima/vis-mcp@1.0.0-alpha.9...@visulima/vis-mcp@1.0.0-alpha.10) (2026-05-14) ### ⚠ BREAKING CHANGES * **vis:** the following security.* keys were renamed: - security.minimumReleaseAge -> security.policies.first_seen.minutes - security.minimumReleaseAgeExclude -> security.policies.first_seen.exclude - security.trustPolicy -> security.policies.publisher_change.mode - security.trustPolicyExclude -> security.policies.publisher_change.exclude - security.trustPolicyIgnoreAfter -> security.policies.publisher_change.ignoreAfter - security.allowBuilds -> security.policies.install_scripts.allow - security.strictDepBuilds -> security.policies.install_scripts.strict - security.socket.minimumScore -> security.policies.score.minimum - security.socket.acceptedRisks -> security.acceptedRisks - security.audit.failOn -> security.policies.vulnerability.failOn - security.audit.usage -> security.policies.vulnerability.usage AcceptedRisk now carries optional policies[] and expiresAt fields so risks can be scoped per-policy and time-boxed. Native PM sync writers keep emitting pnpm-native field names since pnpm owns that schema. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(vis): address coderabbit review feedback - Add JSDoc to exported items across hook builtins, advisories, audit, docker - Document OSV `last_affected` inclusive-upper semantics with a focused test - Annotate NAPI u64→u32 truncations with JS Number range rationale - Switch vis-mcp tool payloads to zod schemas with `.catchall(z.unknown())` so unknown CLI fields stay forward-compatible - Fence RFC code blocks with explicit languages and tidy hook-command formatting (multi-line type, implicit-return arrows) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(vis): address second-round coderabbit feedback on policies rename - Drift report labels now reference `security.policies.*` paths so the hint matches the keys users edit - `defineConfig` JSDoc examples use the new `security.policies` shape - `mergeSecurityDefaults` deep-merges every defaulted sub-policy generically; `mergeVisConfigs` deep-merges `policies` and `acceptedRisks` so presets aren't clobbered - Config-writer scopes its `allow:` match to follow an `install_scripts:` opener - JSDoc clarifications for `audit.advisories.source` default, `acceptedRisks.expiresAt`/`acceptedScore` ranges, `malware.mode` cross-field default, and the current `policies.score.minimum` wiring gap Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(vis): finish coderabbit round-2 fixes — camelCase policies + wire score.minimum - Rename `PolicyName` union to camelCase (`firstSeen`, `installScripts`, `publisherChange`, `unexpectedDeps`). Updates every consumer in src/, test fixtures, schemas, and docs. JSON schema regenerated. - Add `types` mapping to the `#native` package.json import so TypeScript resolves `index.d.ts` when consumers import the alias. - Thread `socketOptions.minimumScore` through `audit`, `doctor`, `check`, `update`, `add`, `formatSecurityOverview`, `formatSummary`, and `applyFilter`. `buildSocketOptions` now resolves the effective minimum once (from `security.policies.score.minimum` or `DEFAULT_LOW_SCORE_THRESHOLD`) so every consumer sees the same value instead of comparing scores against the hard-coded constant. - Update the JSDoc on `policies.score.minimum` to reflect the new wiring. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(vis): add Socket-style policy engine with 4 offline-clean policies Introduce a unified `evaluatePolicies()` engine under `src/security/policies/` with four offline-clean modules: license, install_scripts, vulnerability, and unexpected_deps (baseline mode). Each policy emits PolicyDecisions (block/warn/info) keyed by package, with per-policy accepted-risk scoping and expiresAt support reused from the shared matcher. Wires the engine into `vis audit`: a new `--policies <names>` flag (comma-list, `all`, or `none`) narrows evaluation. Block-severity decisions feed into `--exit-code` and `--fail-on`. JSON output gains `policies[]` + `summary.policyBlocks`. SARIF and HTML formatters render policy decisions alongside vulnerabilities; CSAF and CycloneDX-VEX are intentionally left untouched (vuln-specific data models). Adds `readNodeModulesManifests()` to walk `node_modules/` (including the pnpm `.pnpm/` content-addressed store) and surface license + scripts + maintainers metadata for the offline policies. 33 new unit + integration tests; full vitest suite (3582/3582) green; `tsc --noEmit` clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(vis): close three audit policy gating gaps surfaced by CodeRabbit - applyExitGate's fallthrough call to applyFailOnGate was dropping the policyDecisions argument, so `vis audit --format sarif --fail-on high` exited 0 when the only signal was a block-severity policy decision. Forward the argument. - Unknown --policies tokens were silently swallowed when format was json/sarif/csaf/cyclonedx-vex, so a typoed CI invocation reduced enforcement with no log. Always emit the warning to stderr and surface the tokens in JSON output as `warnings[]`. - Vulnerability-policy block decisions whose advisory was masked by --severity used to exit 1 with no visible reason. Surface those decisions in the human-readable "Policy Decisions" section so the gate is always traceable. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(vis): correct npm/yarn min-release-age native config writers npm CLI types `min-release-age` as `Number` in days, not a duration string — vis wrote `48h`/`15m`, which npm's parseInt would silently read as 48/15 days. Write integer days rounded up so the native gate is never weaker than vis-config. Yarn Berry silently treats day suffixes in `npmMinimalAgeGate` as minutes ### Features * **vis:** offline OSV scanner + unified security.policies ([#632](#632)) ([6461902](6461902)), closes [#631](#631) [#631](#631) [yarnpkg/berry#6991](yarnpkg/berry#6991) ### Dependencies * **@visulima/fs:** upgraded to 5.0.0-alpha.22 * **@visulima/vis:** upgraded to 1.0.0-alpha.20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two-part security upgrade for
vis:vis auditresolves every installed package from the lockfile and matches it against an OSV advisory database held in a local SQLite cache populated byvis advisories sync. No network at scan time, suitable for hermetic CI and airgapped builds. Fix loops (--fix,--fix-transitive) and exit gates (--fail-on,--exit-code) ship alongside.security.policies.*config block (breaking). Eight Socket-style supply-chain knobs (malware, first_seen, unexpected_deps, publisher_change, install_scripts, score, vulnerability, license) consolidate under a singlesecurity.policiesblock. AcceptedRisk gainspolicies[]andexpiresAtso risks can be scoped per-policy and time-boxed.BREAKING CHANGE — config keys renamed
vis is on the alpha channel, so legacy aliases are not kept. Migrate config in place:
security.minimumReleaseAgesecurity.policies.first_seen.minutessecurity.minimumReleaseAgeExcludesecurity.policies.first_seen.excludesecurity.trustPolicysecurity.policies.publisher_change.modesecurity.trustPolicyExcludesecurity.policies.publisher_change.excludesecurity.trustPolicyIgnoreAftersecurity.policies.publisher_change.ignoreAftersecurity.allowBuildssecurity.policies.install_scripts.allowsecurity.strictDepBuildssecurity.policies.install_scripts.strictsecurity.socket.minimumScoresecurity.policies.score.minimumsecurity.socket.acceptedRiskssecurity.acceptedRiskssecurity.audit.failOnsecurity.policies.vulnerability.failOnsecurity.audit.usagesecurity.policies.vulnerability.usageNative PM sync writers (
pnpm-workspace.yaml,.yarnrc.yml,.npmrc,bunfig.toml) keep emitting the pnpm-native field names — that schema is owned upstream.Test plan
tsc --noEmitclean across the package__tests__/security,__tests__/config,__tests__/schemas,__tests__/commands/init,__tests__/commands/doctor) all greenupdate/approve-buildscommand tests pass with renamed keyspnpm exec tsx scripts/generate-schemas.tsaudit,add,install,update,approve-builds,security-auditguide) reflect new keysvis audit --offlineagainst a real workspace🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Configuration
Tests/Benchmarks