From 6daaca1f714e7587b45567774a9abbf3c2a3c3da Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Fri, 4 Sep 2026 13:29:26 +0530 Subject: [PATCH 01/12] Add validate-dependabot-pr Copilot CLI skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a Copilot CLI skill (.github/skills/validate-dependabot-pr.skill.md) that validates open Dependabot npm PRs against Harness Artifact Registry (HAR) compliance, summarizes dependency changes, confirms the ci workflow (validate + build) is green, and verifies the extension still packages and installs correctly. The skill only reports a per-PR merge-readiness verdict; it never approves, comments on, or merges PRs — merging remains a manual step. Signed-off-by: nikhil2611 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../skills/validate-dependabot-pr.skill.md | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 .github/skills/validate-dependabot-pr.skill.md diff --git a/.github/skills/validate-dependabot-pr.skill.md b/.github/skills/validate-dependabot-pr.skill.md new file mode 100644 index 0000000..9e2d763 --- /dev/null +++ b/.github/skills/validate-dependabot-pr.skill.md @@ -0,0 +1,131 @@ +--- +name: validate-dependabot-pr +description: | + Validates open Dependabot npm PRs in chef/vscode-chef against Harness Artifact + Registry (HAR) compliance, reviews the actual dependency change, confirms the + `ci` GitHub workflow (validate + build) is green, and verifies the extension + still packages and installs correctly. Produces a per-PR merge-readiness report. + Never approves, comments on, or merges PRs — a human always merges manually. +instructions: | + You are running the Dependabot PR validation skill for chef/vscode-chef. + + IMPORTANT: You only report. Never run `gh pr merge`, `gh pr review --approve`, or + post PR comments unless the user explicitly asks you to after seeing the report. + + This repo uses **npm only** (no pnpm-workspace.yaml or yarn.lock). Its HAR baseline + lives in the root `.npmrc`: + registry=https://pkg.harness.io/pkg/ct8onj8YTdaXtKaFsYCRLg/org-chef-npm/npm/ + @jsr:registry=https://pkg.harness.io/pkg/ct8onj8YTdaXtKaFsYCRLg/org-chef-npm/npm/ + ignore-scripts=true + min-release-age=14 # stricter than the HAR doc's 7-day default — treat 14 as + # this repo's authoritative floor; never suggest lowering it. + package-lock=true + + When invoked, do the following: + + 1. **Discover open Dependabot PRs** + ```bash + gh pr list --repo chef/vscode-chef --author "app/dependabot" --state open \ + --json number,title,headRefName,headRefOid,url + ``` + If none are open, report that and stop. + + 2. For **each** PR, run all of the following checks: + + a. **HAR compliance check** + `gh pr diff ` does NOT support path filters (`-- ` is rejected with + "accepts at most 1 arg(s)") — always pull the full diff and grep/filter it + via pipes (no temp files needed): + ```bash + gh pr diff --repo chef/vscode-chef | grep -A 20 '^diff --git a/\.npmrc ' + gh pr diff --repo chef/vscode-chef | grep -A 40 '^diff --git a/package\.json ' + gh pr diff --repo chef/vscode-chef | grep '"resolved"' | grep '^+' | grep -v harness.io + ``` + - FAIL if the `.npmrc` hunk removes or weakens any of: the `pkg.harness.io` + registry URL, `@jsr:registry`, `ignore-scripts=true`, `min-release-age=14` + (raising it above 14 is fine; lowering below 14 is not), or + `package-lock=true`. + - FAIL if the third command above prints any added (`+`) `"resolved"` line + not pointing at `pkg.harness.io` — that means the lockfile was + regenerated bypassing HAR (resolved against `registry.npmjs.org`). + - PASS if only version/integrity/resolved-HAR-URL fields changed and the + hardening lines above are untouched (this is the normal, expected case + for routine Dependabot bumps). + + b. **Dependency change check** + - From the same diff, extract: package name(s) changed, old → new version, + whether it's in `dependencies` or `devDependencies`, and whether it's a + direct top-level bump (matches a `package.json` line) or purely transitive + (only appears in `package-lock.json`). + - Flag major-version bumps distinctly from minor/patch — they need closer + human review of changelogs/breaking changes. + + c. **CI workflow check** (`.github/workflows/ci.yml`: jobs `validate`, `build (22.19.0)`) + ```bash + gh pr checks --repo chef/vscode-chef + ``` + This prints one line per check with a `pass`/`fail`/`pending`/`skipping` + status column (e.g. `build (22.19.0) pass 1m6s `). For a JSON view + instead, use `gh pr view --repo chef/vscode-chef --json statusCheckRollup` + (conclusions there are `SUCCESS`/`FAILURE`/etc.). + - PASS only if both the `validate` and `build (22.19.0)` rows show `pass` + (or `SUCCESS` in the JSON form) for the latest commit. Anything else + (`fail`, `pending`, `queued`) is a FAIL — call out which job failed and + include its URL from the output. + + d. **Packaging verification** + - Check out the PR into an isolated git worktree so the user's current + branch/working tree is never disturbed: + ```bash + git fetch origin "pull//head:pr--validate" + git worktree add /tmp/vscode-chef-pr- pr--validate + cd /tmp/vscode-chef-pr- + ``` + - Run the same steps as the `build` CI job: + ```bash + npm ci + npx vsce package --out /tmp/vscode-chef-pr-.vsix + ``` + - PASS only if both commands exit 0 and the `.vsix` file exists with a + reasonable size (roughly comparable to the current `vscode-chef.vsix` in + the repo root, not near-zero). + + e. **Installation verification** + - Unzip the produced `.vsix` (it's a zip archive): + ```bash + unzip -p /tmp/vscode-chef-pr-.vsix extension/package.json > /tmp/pr--manifest.json + ``` + - Confirm `/tmp/pr--manifest.json` is valid JSON, its `version` matches + `package.json` on the PR branch, and required fields (`engines.vscode`, + `main`, `contributes`) are present and structurally unchanged from `main`. + - If a `code` CLI is available in this environment, optionally try: + ```bash + code --install-extension /tmp/vscode-chef-pr-.vsix + ``` + as a stronger signal. If `code` is not available, skip this step without + failing the PR for that reason alone — note it as "not checked (no code CLI)". + - **Always clean up** after each PR, even on failure, so no scratch state + leaks between PRs or back into the user's main checkout: + ```bash + cd /Users/ngupta/Documents/chef-workstation-hab-pkg/rel/CW26/vscode-chef + git worktree remove /tmp/vscode-chef-pr- --force + git branch -D pr--validate + rm -f /tmp/vscode-chef-pr-.vsix /tmp/pr--manifest.json + ``` + + 3. **Report** + Print one summary table across all open Dependabot PRs, one row per PR: + + | PR # | Title | HAR | Deps Changed | CI (validate/build) | Package | Install | Verdict | + |------|-------|-----|---------------|----------------------|---------|---------|---------| + | 285 | Bump @types/vscode 1.83.3→1.134.0 | ✅ | @types/vscode (dev, minor) | ✅ / ❌ build failed | ⚠️ not run (CI red) | ⚠️ not run | ⚠️ Needs attention: build job failing | + + - Verdict is **"✅ Ready to merge manually"** only when every check passes. + - Otherwise **"⚠️ Needs attention"** with the specific failing check(s) named. + - If CI is already failing for a PR, it's fine to skip the local packaging and + installation steps for that PR (note as "not run — CI red") rather than spend + time re-deriving a failure GitHub already reported. Still run the packaging + and installation steps locally whenever CI is green, or whenever the user + explicitly asks you to double-check a CI failure yourself. + - Do not approve, comment on, or merge any PR. End the report by reminding the + user that merging is manual, per team policy. From 8b2c569192cc76a652a0a02d053ba2303206c57b Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Fri, 4 Sep 2026 13:35:19 +0530 Subject: [PATCH 02/12] Address review feedback: capture REPO_ROOT for worktree cleanup The packaging-verification steps in the skill instructions cd into a scratch /tmp worktree but never captured the original repo directory, forcing the cleanup step to hardcode a developer-specific path. Capture `REPO_ROOT="$(pwd)"` before entering the worktree and use it in the cleanup step instead, so the instructions work for any checkout path. Signed-off-by: nikhil2611 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/skills/validate-dependabot-pr.skill.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/skills/validate-dependabot-pr.skill.md b/.github/skills/validate-dependabot-pr.skill.md index 9e2d763..5093ec0 100644 --- a/.github/skills/validate-dependabot-pr.skill.md +++ b/.github/skills/validate-dependabot-pr.skill.md @@ -75,8 +75,11 @@ instructions: | d. **Packaging verification** - Check out the PR into an isolated git worktree so the user's current - branch/working tree is never disturbed: + branch/working tree is never disturbed. Capture the original repo + directory first so cleanup can reliably return to it (don't hardcode + a developer-specific path): ```bash + REPO_ROOT="$(pwd)" git fetch origin "pull//head:pr--validate" git worktree add /tmp/vscode-chef-pr- pr--validate cd /tmp/vscode-chef-pr- @@ -105,9 +108,11 @@ instructions: | as a stronger signal. If `code` is not available, skip this step without failing the PR for that reason alone — note it as "not checked (no code CLI)". - **Always clean up** after each PR, even on failure, so no scratch state - leaks between PRs or back into the user's main checkout: + leaks between PRs or back into the user's main checkout. Use the + `REPO_ROOT` captured before entering the worktree — never hardcode a + path: ```bash - cd /Users/ngupta/Documents/chef-workstation-hab-pkg/rel/CW26/vscode-chef + cd "$REPO_ROOT" git worktree remove /tmp/vscode-chef-pr- --force git branch -D pr--validate rm -f /tmp/vscode-chef-pr-.vsix /tmp/pr--manifest.json From bd1e9a7539ef43b4f758ebd268347061a45ee271 Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Fri, 4 Sep 2026 13:41:58 +0530 Subject: [PATCH 03/12] Address Copilot review comments on validate-dependabot-pr skill - Derive HAR host dynamically from .npmrc's registry= line instead of hardcoding pkg.harness.io, so the resolved-URL filter can't be fooled by other *.harness.io hosts. - Replace non-existent vscode-chef.vsix baseline size comparison with a concrete, portable non-empty/>100KB sanity check for the packaged .vsix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: nikhil2611 --- .../skills/validate-dependabot-pr.skill.md | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/.github/skills/validate-dependabot-pr.skill.md b/.github/skills/validate-dependabot-pr.skill.md index 5093ec0..56f47f0 100644 --- a/.github/skills/validate-dependabot-pr.skill.md +++ b/.github/skills/validate-dependabot-pr.skill.md @@ -35,19 +35,22 @@ instructions: | a. **HAR compliance check** `gh pr diff ` does NOT support path filters (`-- ` is rejected with "accepts at most 1 arg(s)") — always pull the full diff and grep/filter it - via pipes (no temp files needed): + via pipes (no temp files needed). Derive the HAR host from this repo's own + `.npmrc` rather than hardcoding it, so the check stays correct if the org + path or host ever changes: ```bash + HAR_HOST="$(grep '^registry=' .npmrc | sed -E 's#^registry=https?://([^/]+)/.*#\1#')" gh pr diff --repo chef/vscode-chef | grep -A 20 '^diff --git a/\.npmrc ' gh pr diff --repo chef/vscode-chef | grep -A 40 '^diff --git a/package\.json ' - gh pr diff --repo chef/vscode-chef | grep '"resolved"' | grep '^+' | grep -v harness.io + gh pr diff --repo chef/vscode-chef | grep '"resolved"' | grep '^+' | grep -v "$HAR_HOST" ``` - - FAIL if the `.npmrc` hunk removes or weakens any of: the `pkg.harness.io` + - FAIL if the `.npmrc` hunk removes or weakens any of: the `$HAR_HOST` registry URL, `@jsr:registry`, `ignore-scripts=true`, `min-release-age=14` (raising it above 14 is fine; lowering below 14 is not), or `package-lock=true`. - FAIL if the third command above prints any added (`+`) `"resolved"` line - not pointing at `pkg.harness.io` — that means the lockfile was - regenerated bypassing HAR (resolved against `registry.npmjs.org`). + not pointing at `$HAR_HOST` — that means the lockfile was regenerated + bypassing HAR (resolved against `registry.npmjs.org`). - PASS if only version/integrity/resolved-HAR-URL fields changed and the hardening lines above are untouched (this is the normal, expected case for routine Dependabot bumps). @@ -89,9 +92,10 @@ instructions: | npm ci npx vsce package --out /tmp/vscode-chef-pr-.vsix ``` - - PASS only if both commands exit 0 and the `.vsix` file exists with a - reasonable size (roughly comparable to the current `vscode-chef.vsix` in - the repo root, not near-zero). + - PASS only if both commands exit 0 and the `.vsix` file exists and is not + suspiciously small (e.g. non-empty and comfortably above a minimal + threshold such as 100KB, via `stat -f%z` on macOS or `stat -c%s` on Linux + against `/tmp/vscode-chef-pr-.vsix`). e. **Installation verification** - Unzip the produced `.vsix` (it's a zip archive): From 3a169702a02c6de57f5961fbc2b042d555d08be9 Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Fri, 4 Sep 2026 14:06:35 +0530 Subject: [PATCH 04/12] Make worktree fetch/cleanup steps more robust - Fetch PR ref from the canonical chef/vscode-chef URL instead of the origin remote, since a developer local origin may point at a personal fork where pull//head would not resolve. - Suffix worktree/branch cleanup commands with || true so a partial earlier failure (worktree or branch never created) does not abort the rest of cleanup and leave scratch state behind. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: nikhil2611 --- .github/skills/validate-dependabot-pr.skill.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/skills/validate-dependabot-pr.skill.md b/.github/skills/validate-dependabot-pr.skill.md index 56f47f0..e9a0198 100644 --- a/.github/skills/validate-dependabot-pr.skill.md +++ b/.github/skills/validate-dependabot-pr.skill.md @@ -83,10 +83,13 @@ instructions: | a developer-specific path): ```bash REPO_ROOT="$(pwd)" - git fetch origin "pull//head:pr--validate" + git fetch https://github.com/chef/vscode-chef.git "pull//head:pr--validate" git worktree add /tmp/vscode-chef-pr- pr--validate cd /tmp/vscode-chef-pr- ``` + Fetch from the canonical `chef/vscode-chef` URL explicitly rather than + `origin` — a developer's local `origin` may point at a personal fork, + in which case `pull//head` wouldn't resolve there. - Run the same steps as the `build` CI job: ```bash npm ci @@ -114,11 +117,13 @@ instructions: | - **Always clean up** after each PR, even on failure, so no scratch state leaks between PRs or back into the user's main checkout. Use the `REPO_ROOT` captured before entering the worktree — never hardcode a - path: + path. Each step is tolerant of partial failure (e.g. an earlier step in + this same check aborted before the worktree/branch/file existed), so + suffix with `|| true` and keep going rather than stopping cleanup short: ```bash cd "$REPO_ROOT" - git worktree remove /tmp/vscode-chef-pr- --force - git branch -D pr--validate + git worktree remove /tmp/vscode-chef-pr- --force || true + git branch -D pr--validate || true rm -f /tmp/vscode-chef-pr-.vsix /tmp/pr--manifest.json ``` From 5c8429279bdf2f37015516232ffb5f4a293742b3 Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Fri, 4 Sep 2026 14:15:34 +0530 Subject: [PATCH 05/12] Restructure skill to proper Copilot CLI SKILL.md format Copilot review flagged that the skill file's YAML front matter was never closed with a second `---`, and the operational instructions were nested under an `instructions:` YAML key instead of being a plain Markdown body. Per GitHub's documented format, Copilot CLI skills must live in `.github/skills//SKILL.md`, with minimal delimited YAML front matter (name, description only) followed by a Markdown body containing the instructions. Moved `.github/skills/validate-dependabot-pr.skill.md` to `.github/skills/validate-dependabot-pr/SKILL.md`, converting the `instructions:` YAML block scalar into a normal Markdown body with proper heading structure, and folding the multi-line `description:` into a single flowed line as required by the frontmatter spec. Signed-off-by: nikhil2611 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../skills/validate-dependabot-pr.skill.md | 145 --------------- .../skills/validate-dependabot-pr/SKILL.md | 167 ++++++++++++++++++ 2 files changed, 167 insertions(+), 145 deletions(-) delete mode 100644 .github/skills/validate-dependabot-pr.skill.md create mode 100644 .github/skills/validate-dependabot-pr/SKILL.md diff --git a/.github/skills/validate-dependabot-pr.skill.md b/.github/skills/validate-dependabot-pr.skill.md deleted file mode 100644 index e9a0198..0000000 --- a/.github/skills/validate-dependabot-pr.skill.md +++ /dev/null @@ -1,145 +0,0 @@ ---- -name: validate-dependabot-pr -description: | - Validates open Dependabot npm PRs in chef/vscode-chef against Harness Artifact - Registry (HAR) compliance, reviews the actual dependency change, confirms the - `ci` GitHub workflow (validate + build) is green, and verifies the extension - still packages and installs correctly. Produces a per-PR merge-readiness report. - Never approves, comments on, or merges PRs — a human always merges manually. -instructions: | - You are running the Dependabot PR validation skill for chef/vscode-chef. - - IMPORTANT: You only report. Never run `gh pr merge`, `gh pr review --approve`, or - post PR comments unless the user explicitly asks you to after seeing the report. - - This repo uses **npm only** (no pnpm-workspace.yaml or yarn.lock). Its HAR baseline - lives in the root `.npmrc`: - registry=https://pkg.harness.io/pkg/ct8onj8YTdaXtKaFsYCRLg/org-chef-npm/npm/ - @jsr:registry=https://pkg.harness.io/pkg/ct8onj8YTdaXtKaFsYCRLg/org-chef-npm/npm/ - ignore-scripts=true - min-release-age=14 # stricter than the HAR doc's 7-day default — treat 14 as - # this repo's authoritative floor; never suggest lowering it. - package-lock=true - - When invoked, do the following: - - 1. **Discover open Dependabot PRs** - ```bash - gh pr list --repo chef/vscode-chef --author "app/dependabot" --state open \ - --json number,title,headRefName,headRefOid,url - ``` - If none are open, report that and stop. - - 2. For **each** PR, run all of the following checks: - - a. **HAR compliance check** - `gh pr diff ` does NOT support path filters (`-- ` is rejected with - "accepts at most 1 arg(s)") — always pull the full diff and grep/filter it - via pipes (no temp files needed). Derive the HAR host from this repo's own - `.npmrc` rather than hardcoding it, so the check stays correct if the org - path or host ever changes: - ```bash - HAR_HOST="$(grep '^registry=' .npmrc | sed -E 's#^registry=https?://([^/]+)/.*#\1#')" - gh pr diff --repo chef/vscode-chef | grep -A 20 '^diff --git a/\.npmrc ' - gh pr diff --repo chef/vscode-chef | grep -A 40 '^diff --git a/package\.json ' - gh pr diff --repo chef/vscode-chef | grep '"resolved"' | grep '^+' | grep -v "$HAR_HOST" - ``` - - FAIL if the `.npmrc` hunk removes or weakens any of: the `$HAR_HOST` - registry URL, `@jsr:registry`, `ignore-scripts=true`, `min-release-age=14` - (raising it above 14 is fine; lowering below 14 is not), or - `package-lock=true`. - - FAIL if the third command above prints any added (`+`) `"resolved"` line - not pointing at `$HAR_HOST` — that means the lockfile was regenerated - bypassing HAR (resolved against `registry.npmjs.org`). - - PASS if only version/integrity/resolved-HAR-URL fields changed and the - hardening lines above are untouched (this is the normal, expected case - for routine Dependabot bumps). - - b. **Dependency change check** - - From the same diff, extract: package name(s) changed, old → new version, - whether it's in `dependencies` or `devDependencies`, and whether it's a - direct top-level bump (matches a `package.json` line) or purely transitive - (only appears in `package-lock.json`). - - Flag major-version bumps distinctly from minor/patch — they need closer - human review of changelogs/breaking changes. - - c. **CI workflow check** (`.github/workflows/ci.yml`: jobs `validate`, `build (22.19.0)`) - ```bash - gh pr checks --repo chef/vscode-chef - ``` - This prints one line per check with a `pass`/`fail`/`pending`/`skipping` - status column (e.g. `build (22.19.0) pass 1m6s `). For a JSON view - instead, use `gh pr view --repo chef/vscode-chef --json statusCheckRollup` - (conclusions there are `SUCCESS`/`FAILURE`/etc.). - - PASS only if both the `validate` and `build (22.19.0)` rows show `pass` - (or `SUCCESS` in the JSON form) for the latest commit. Anything else - (`fail`, `pending`, `queued`) is a FAIL — call out which job failed and - include its URL from the output. - - d. **Packaging verification** - - Check out the PR into an isolated git worktree so the user's current - branch/working tree is never disturbed. Capture the original repo - directory first so cleanup can reliably return to it (don't hardcode - a developer-specific path): - ```bash - REPO_ROOT="$(pwd)" - git fetch https://github.com/chef/vscode-chef.git "pull//head:pr--validate" - git worktree add /tmp/vscode-chef-pr- pr--validate - cd /tmp/vscode-chef-pr- - ``` - Fetch from the canonical `chef/vscode-chef` URL explicitly rather than - `origin` — a developer's local `origin` may point at a personal fork, - in which case `pull//head` wouldn't resolve there. - - Run the same steps as the `build` CI job: - ```bash - npm ci - npx vsce package --out /tmp/vscode-chef-pr-.vsix - ``` - - PASS only if both commands exit 0 and the `.vsix` file exists and is not - suspiciously small (e.g. non-empty and comfortably above a minimal - threshold such as 100KB, via `stat -f%z` on macOS or `stat -c%s` on Linux - against `/tmp/vscode-chef-pr-.vsix`). - - e. **Installation verification** - - Unzip the produced `.vsix` (it's a zip archive): - ```bash - unzip -p /tmp/vscode-chef-pr-.vsix extension/package.json > /tmp/pr--manifest.json - ``` - - Confirm `/tmp/pr--manifest.json` is valid JSON, its `version` matches - `package.json` on the PR branch, and required fields (`engines.vscode`, - `main`, `contributes`) are present and structurally unchanged from `main`. - - If a `code` CLI is available in this environment, optionally try: - ```bash - code --install-extension /tmp/vscode-chef-pr-.vsix - ``` - as a stronger signal. If `code` is not available, skip this step without - failing the PR for that reason alone — note it as "not checked (no code CLI)". - - **Always clean up** after each PR, even on failure, so no scratch state - leaks between PRs or back into the user's main checkout. Use the - `REPO_ROOT` captured before entering the worktree — never hardcode a - path. Each step is tolerant of partial failure (e.g. an earlier step in - this same check aborted before the worktree/branch/file existed), so - suffix with `|| true` and keep going rather than stopping cleanup short: - ```bash - cd "$REPO_ROOT" - git worktree remove /tmp/vscode-chef-pr- --force || true - git branch -D pr--validate || true - rm -f /tmp/vscode-chef-pr-.vsix /tmp/pr--manifest.json - ``` - - 3. **Report** - Print one summary table across all open Dependabot PRs, one row per PR: - - | PR # | Title | HAR | Deps Changed | CI (validate/build) | Package | Install | Verdict | - |------|-------|-----|---------------|----------------------|---------|---------|---------| - | 285 | Bump @types/vscode 1.83.3→1.134.0 | ✅ | @types/vscode (dev, minor) | ✅ / ❌ build failed | ⚠️ not run (CI red) | ⚠️ not run | ⚠️ Needs attention: build job failing | - - - Verdict is **"✅ Ready to merge manually"** only when every check passes. - - Otherwise **"⚠️ Needs attention"** with the specific failing check(s) named. - - If CI is already failing for a PR, it's fine to skip the local packaging and - installation steps for that PR (note as "not run — CI red") rather than spend - time re-deriving a failure GitHub already reported. Still run the packaging - and installation steps locally whenever CI is green, or whenever the user - explicitly asks you to double-check a CI failure yourself. - - Do not approve, comment on, or merge any PR. End the report by reminding the - user that merging is manual, per team policy. diff --git a/.github/skills/validate-dependabot-pr/SKILL.md b/.github/skills/validate-dependabot-pr/SKILL.md new file mode 100644 index 0000000..86d7166 --- /dev/null +++ b/.github/skills/validate-dependabot-pr/SKILL.md @@ -0,0 +1,167 @@ +--- +name: validate-dependabot-pr +description: Validates open Dependabot npm PRs in chef/vscode-chef against Harness Artifact Registry (HAR) compliance, reviews the actual dependency change, confirms the ci GitHub workflow (validate + build) is green, and verifies the extension still packages and installs correctly. Produces a per-PR merge-readiness report. Never approves, comments on, or merges PRs — a human always merges manually. Use this when asked to validate, review, or check open Dependabot PRs in vscode-chef before merging. +--- + +You are running the Dependabot PR validation skill for chef/vscode-chef. + +IMPORTANT: You only report. Never run `gh pr merge`, `gh pr review --approve`, or +post PR comments unless the user explicitly asks you to after seeing the report. + +This repo uses **npm only** (no pnpm-workspace.yaml or yarn.lock). Its HAR baseline +lives in the root `.npmrc`: +``` +registry=https://pkg.harness.io/pkg/ct8onj8YTdaXtKaFsYCRLg/org-chef-npm/npm/ +@jsr:registry=https://pkg.harness.io/pkg/ct8onj8YTdaXtKaFsYCRLg/org-chef-npm/npm/ +ignore-scripts=true +min-release-age=14 # stricter than the HAR doc's 7-day default — treat 14 as + # this repo's authoritative floor; never suggest lowering it. +package-lock=true +``` + +When invoked, do the following: + +## 1. Discover open Dependabot PRs + +```bash +gh pr list --repo chef/vscode-chef --author "app/dependabot" --state open \ + --json number,title,headRefName,headRefOid,url +``` + +If none are open, report that and stop. + +## 2. For each PR, run all of the following checks + +### a. HAR compliance check + +`gh pr diff ` does NOT support path filters (`-- ` is rejected with +"accepts at most 1 arg(s)") — always pull the full diff and grep/filter it +via pipes (no temp files needed). Derive the HAR host from this repo's own +`.npmrc` rather than hardcoding it, so the check stays correct if the org +path or host ever changes: + +```bash +HAR_HOST="$(grep '^registry=' .npmrc | sed -E 's#^registry=https?://([^/]+)/.*#\1#')" +gh pr diff --repo chef/vscode-chef | grep -A 20 '^diff --git a/\.npmrc ' +gh pr diff --repo chef/vscode-chef | grep -A 40 '^diff --git a/package\.json ' +gh pr diff --repo chef/vscode-chef | grep '"resolved"' | grep '^+' | grep -v "$HAR_HOST" +``` + +- FAIL if the `.npmrc` hunk removes or weakens any of: the `$HAR_HOST` + registry URL, `@jsr:registry`, `ignore-scripts=true`, `min-release-age=14` + (raising it above 14 is fine; lowering below 14 is not), or + `package-lock=true`. +- FAIL if the third command above prints any added (`+`) `"resolved"` line + not pointing at `$HAR_HOST` — that means the lockfile was regenerated + bypassing HAR (resolved against `registry.npmjs.org`). +- PASS if only version/integrity/resolved-HAR-URL fields changed and the + hardening lines above are untouched (this is the normal, expected case + for routine Dependabot bumps). + +### b. Dependency change check + +- From the same diff, extract: package name(s) changed, old → new version, + whether it's in `dependencies` or `devDependencies`, and whether it's a + direct top-level bump (matches a `package.json` line) or purely transitive + (only appears in `package-lock.json`). +- Flag major-version bumps distinctly from minor/patch — they need closer + human review of changelogs/breaking changes. + +### c. CI workflow check (`.github/workflows/ci.yml`: jobs `validate`, `build (22.19.0)`) + +```bash +gh pr checks --repo chef/vscode-chef +``` + +This prints one line per check with a `pass`/`fail`/`pending`/`skipping` +status column (e.g. `build (22.19.0) pass 1m6s `). For a JSON view +instead, use `gh pr view --repo chef/vscode-chef --json statusCheckRollup` +(conclusions there are `SUCCESS`/`FAILURE`/etc.). + +- PASS only if both the `validate` and `build (22.19.0)` rows show `pass` + (or `SUCCESS` in the JSON form) for the latest commit. Anything else + (`fail`, `pending`, `queued`) is a FAIL — call out which job failed and + include its URL from the output. + +### d. Packaging verification + +Check out the PR into an isolated git worktree so the user's current +branch/working tree is never disturbed. Capture the original repo +directory first so cleanup can reliably return to it (don't hardcode +a developer-specific path): + +```bash +REPO_ROOT="$(pwd)" +git fetch https://github.com/chef/vscode-chef.git "pull//head:pr--validate" +git worktree add /tmp/vscode-chef-pr- pr--validate +cd /tmp/vscode-chef-pr- +``` + +Fetch from the canonical `chef/vscode-chef` URL explicitly rather than +`origin` — a developer's local `origin` may point at a personal fork, +in which case `pull//head` wouldn't resolve there. + +Run the same steps as the `build` CI job: + +```bash +npm ci +npx vsce package --out /tmp/vscode-chef-pr-.vsix +``` + +PASS only if both commands exit 0 and the `.vsix` file exists and is not +suspiciously small (e.g. non-empty and comfortably above a minimal +threshold such as 100KB, via `stat -f%z` on macOS or `stat -c%s` on Linux +against `/tmp/vscode-chef-pr-.vsix`). + +### e. Installation verification + +Unzip the produced `.vsix` (it's a zip archive): + +```bash +unzip -p /tmp/vscode-chef-pr-.vsix extension/package.json > /tmp/pr--manifest.json +``` + +Confirm `/tmp/pr--manifest.json` is valid JSON, its `version` matches +`package.json` on the PR branch, and required fields (`engines.vscode`, +`main`, `contributes`) are present and structurally unchanged from `main`. + +If a `code` CLI is available in this environment, optionally try: + +```bash +code --install-extension /tmp/vscode-chef-pr-.vsix +``` + +as a stronger signal. If `code` is not available, skip this step without +failing the PR for that reason alone — note it as "not checked (no code CLI)". + +**Always clean up** after each PR, even on failure, so no scratch state +leaks between PRs or back into the user's main checkout. Use the +`REPO_ROOT` captured before entering the worktree — never hardcode a +path. Each step is tolerant of partial failure (e.g. an earlier step in +this same check aborted before the worktree/branch/file existed), so +suffix with `|| true` and keep going rather than stopping cleanup short: + +```bash +cd "$REPO_ROOT" +git worktree remove /tmp/vscode-chef-pr- --force || true +git branch -D pr--validate || true +rm -f /tmp/vscode-chef-pr-.vsix /tmp/pr--manifest.json +``` + +## 3. Report + +Print one summary table across all open Dependabot PRs, one row per PR: + +| PR # | Title | HAR | Deps Changed | CI (validate/build) | Package | Install | Verdict | +|------|-------|-----|---------------|----------------------|---------|---------|---------| +| 285 | Bump @types/vscode 1.83.3→1.134.0 | ✅ | @types/vscode (dev, minor) | ✅ / ❌ build failed | ⚠️ not run (CI red) | ⚠️ not run | ⚠️ Needs attention: build job failing | + +- Verdict is **"✅ Ready to merge manually"** only when every check passes. +- Otherwise **"⚠️ Needs attention"** with the specific failing check(s) named. +- If CI is already failing for a PR, it's fine to skip the local packaging and + installation steps for that PR (note as "not run — CI red") rather than spend + time re-deriving a failure GitHub already reported. Still run the packaging + and installation steps locally whenever CI is green, or whenever the user + explicitly asks you to double-check a CI failure yourself. +- Do not approve, comment on, or merge any PR. End the report by reminding the + user that merging is manual, per team policy. From ee6c94477dddc25860dc63484bbe38a7fea77725 Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Fri, 4 Sep 2026 14:21:17 +0530 Subject: [PATCH 06/12] Address Copilot review: guard HAR_HOST, force fetch refspec - Add an explicit non-empty guard for HAR_HOST before running the diff greps. Previously, if HAR_HOST failed to parse (e.g. .npmrc missing or edited locally), `grep -v "$HAR_HOST"` with an empty pattern would suppress all output, making the HAR bypass check silently false-negative (always appear clean). Now the check exits with an error instead. - Use a forced refspec (`+pull//head:...`) when fetching the PR branch into a local ref for packaging verification, so the fetch reliably updates the local branch even when Dependabot force-pushes (rebases or refreshes) an existing PR between validation runs. The PR description was also updated separately to reference the correct file path (.github/skills/validate-dependabot-pr/SKILL.md) after the previous restructuring commit, since it still described the old flat .skill.md location. Signed-off-by: nikhil2611 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/skills/validate-dependabot-pr/SKILL.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/skills/validate-dependabot-pr/SKILL.md b/.github/skills/validate-dependabot-pr/SKILL.md index 86d7166..b081ff2 100644 --- a/.github/skills/validate-dependabot-pr/SKILL.md +++ b/.github/skills/validate-dependabot-pr/SKILL.md @@ -42,11 +42,16 @@ path or host ever changes: ```bash HAR_HOST="$(grep '^registry=' .npmrc | sed -E 's#^registry=https?://([^/]+)/.*#\1#')" +if [ -z "$HAR_HOST" ]; then echo "ERROR: could not derive HAR_HOST from .npmrc" >&2; exit 1; fi gh pr diff --repo chef/vscode-chef | grep -A 20 '^diff --git a/\.npmrc ' gh pr diff --repo chef/vscode-chef | grep -A 40 '^diff --git a/package\.json ' gh pr diff --repo chef/vscode-chef | grep '"resolved"' | grep '^+' | grep -v "$HAR_HOST" ``` +- **Never skip this check if `HAR_HOST` is empty** — an empty pattern passed to + `grep -v` would suppress all output and make the bypass check falsely appear + clean. The `exit 1` guard above prevents this; if it fires, stop and report + that HAR compliance could not be verified rather than assuming a pass. - FAIL if the `.npmrc` hunk removes or weakens any of: the `$HAR_HOST` registry URL, `@jsr:registry`, `ignore-scripts=true`, `min-release-age=14` (raising it above 14 is fine; lowering below 14 is not), or @@ -92,14 +97,17 @@ a developer-specific path): ```bash REPO_ROOT="$(pwd)" -git fetch https://github.com/chef/vscode-chef.git "pull//head:pr--validate" +git fetch https://github.com/chef/vscode-chef.git "+pull//head:pr--validate" git worktree add /tmp/vscode-chef-pr- pr--validate cd /tmp/vscode-chef-pr- ``` Fetch from the canonical `chef/vscode-chef` URL explicitly rather than `origin` — a developer's local `origin` may point at a personal fork, -in which case `pull//head` wouldn't resolve there. +in which case `pull//head` wouldn't resolve there. Use the forced +refspec (`+pull//head:...`) so the fetch reliably updates the local +branch even if Dependabot has force-pushed (rebased/refreshed) the PR +since a previous validation run. Run the same steps as the `build` CI job: From 8da38c85ab74fd4b2789ce903a7c25e2f5454f32 Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Mon, 7 Sep 2026 14:25:04 +0530 Subject: [PATCH 07/12] Fix HAR grep to use fixed-string matching and make CI check name matching resilient to Node version bumps - grep -v "$HAR_HOST" treated dots as regex wildcards, risking false negatives if a non-HAR host happened to match the pattern. Use grep -F -v -- for literal string matching. - The CI check hardcoded 'build (22.19.0)', which breaks silently whenever the workflow's Node matrix version changes. Match the 'build (' prefix instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: nikhil2611 --- .../skills/validate-dependabot-pr/SKILL.md | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/.github/skills/validate-dependabot-pr/SKILL.md b/.github/skills/validate-dependabot-pr/SKILL.md index b081ff2..493ecfb 100644 --- a/.github/skills/validate-dependabot-pr/SKILL.md +++ b/.github/skills/validate-dependabot-pr/SKILL.md @@ -45,7 +45,7 @@ HAR_HOST="$(grep '^registry=' .npmrc | sed -E 's#^registry=https?://([^/]+)/.*#\ if [ -z "$HAR_HOST" ]; then echo "ERROR: could not derive HAR_HOST from .npmrc" >&2; exit 1; fi gh pr diff --repo chef/vscode-chef | grep -A 20 '^diff --git a/\.npmrc ' gh pr diff --repo chef/vscode-chef | grep -A 40 '^diff --git a/package\.json ' -gh pr diff --repo chef/vscode-chef | grep '"resolved"' | grep '^+' | grep -v "$HAR_HOST" +gh pr diff --repo chef/vscode-chef | grep '"resolved"' | grep '^+' | grep -F -v -- "$HAR_HOST" ``` - **Never skip this check if `HAR_HOST` is empty** — an empty pattern passed to @@ -72,7 +72,13 @@ gh pr diff --repo chef/vscode-chef | grep '"resolved"' | grep '^+' | grep -v - Flag major-version bumps distinctly from minor/patch — they need closer human review of changelogs/breaking changes. -### c. CI workflow check (`.github/workflows/ci.yml`: jobs `validate`, `build (22.19.0)`) +### c. CI workflow check (`.github/workflows/ci.yml`: jobs `validate`, `build ()`) + +The `build` job name embeds the Node version from the workflow's matrix +(currently `22.19.0`), so never hardcode `build (22.19.0)` — that string +breaks silently the next time the matrix version is bumped. Instead match +the `build (` prefix, and optionally cross-check it against the workflow +file's current matrix value: ```bash gh pr checks --repo chef/vscode-chef @@ -83,10 +89,11 @@ status column (e.g. `build (22.19.0) pass 1m6s `). For a JSON view instead, use `gh pr view --repo chef/vscode-chef --json statusCheckRollup` (conclusions there are `SUCCESS`/`FAILURE`/etc.). -- PASS only if both the `validate` and `build (22.19.0)` rows show `pass` - (or `SUCCESS` in the JSON form) for the latest commit. Anything else - (`fail`, `pending`, `queued`) is a FAIL — call out which job failed and - include its URL from the output. +- PASS only if the `validate` row and the row whose name starts with + `build (` both show `pass` (or `SUCCESS` in the JSON form) for the latest + commit. Anything else (`fail`, `pending`, `queued`, or a missing `build (` + row) is a FAIL — call out which job failed and include its URL from the + output. ### d. Packaging verification From cbed1f19886ed9636e307351ce4a8627ef0ca3de Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Mon, 7 Sep 2026 14:31:21 +0530 Subject: [PATCH 08/12] Make HAR diff extraction robust and worktree setup idempotent - Replace grep -A with sed range extraction for .npmrc/package.json diff hunks so full hunks are captured regardless of length, instead of being cut off at a fixed line count. - Scope the resolved-URL bypass check to the package-lock.json diff section only, and append || true so a clean result (no non-HAR resolved lines) doesn't cause the pipeline to exit non-zero. - Pre-clean any lingering worktree/branch from a prior run before fetching, so repeated invocations of the skill don't fail on an already-checked-out branch or existing worktree directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: nikhil2611 --- .../skills/validate-dependabot-pr/SKILL.md | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/.github/skills/validate-dependabot-pr/SKILL.md b/.github/skills/validate-dependabot-pr/SKILL.md index 493ecfb..fe29dbc 100644 --- a/.github/skills/validate-dependabot-pr/SKILL.md +++ b/.github/skills/validate-dependabot-pr/SKILL.md @@ -43,11 +43,20 @@ path or host ever changes: ```bash HAR_HOST="$(grep '^registry=' .npmrc | sed -E 's#^registry=https?://([^/]+)/.*#\1#')" if [ -z "$HAR_HOST" ]; then echo "ERROR: could not derive HAR_HOST from .npmrc" >&2; exit 1; fi -gh pr diff --repo chef/vscode-chef | grep -A 20 '^diff --git a/\.npmrc ' -gh pr diff --repo chef/vscode-chef | grep -A 40 '^diff --git a/package\.json ' -gh pr diff --repo chef/vscode-chef | grep '"resolved"' | grep '^+' | grep -F -v -- "$HAR_HOST" +gh pr diff --repo chef/vscode-chef | sed -n '/^diff --git a\/\.npmrc /,/^diff --git /p' +gh pr diff --repo chef/vscode-chef | sed -n '/^diff --git a\/package\.json /,/^diff --git /p' +gh pr diff --repo chef/vscode-chef | sed -n '/^diff --git a\/package-lock\.json /,/^diff --git /p' | grep '"resolved"' | grep '^+' | grep -F -v -- "$HAR_HOST" || true ``` +Use `sed` ranges (not `grep -A `) so the full `.npmrc`/`package.json` +diff hunks are captured regardless of how many lines they span, and scope +the resolved-URL bypass check to the `package-lock.json` diff section +specifically (that's the only file where `"resolved"` lines matter). Append +`|| true` to the last pipeline — under `set -e` (or when this snippet's +exit status is checked), a `grep -v` that matches nothing exits non-zero, +and "no non-HAR resolved lines found" must be treated as a clean pass, not +a script failure. + - **Never skip this check if `HAR_HOST` is empty** — an empty pattern passed to `grep -v` would suppress all output and make the bypass check falsely appear clean. The `exit 1` guard above prevents this; if it fires, stop and report @@ -104,6 +113,8 @@ a developer-specific path): ```bash REPO_ROOT="$(pwd)" +git worktree remove /tmp/vscode-chef-pr- --force || true +git branch -D pr--validate || true git fetch https://github.com/chef/vscode-chef.git "+pull//head:pr--validate" git worktree add /tmp/vscode-chef-pr- pr--validate cd /tmp/vscode-chef-pr- @@ -116,6 +127,13 @@ refspec (`+pull//head:...`) so the fetch reliably updates the local branch even if Dependabot has force-pushed (rebased/refreshed) the PR since a previous validation run. +Run the pre-clean (`git worktree remove` / `git branch -D`, both tolerant +of failure via `|| true`) before the fetch so re-running this skill is +idempotent: a lingering worktree directory or an already-checked-out +`pr--validate` branch from a prior/interrupted run would otherwise +cause `git fetch` to refuse updating a checked-out branch, or +`git worktree add` to fail on an existing directory. + Run the same steps as the `build` CI job: ```bash From 651b3c232911c5840661ec25317a5ea63e06bacf Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Mon, 7 Sep 2026 14:38:29 +0530 Subject: [PATCH 09/12] Fix inaccurate .npmrc example and dedupe repeated gh pr diff calls - Align the .npmrc example in the skill doc with the repo's actual file: use standalone comment lines instead of an inline/continuation comment on min-release-age=14, preventing confusing copy/paste. - Fetch the PR diff once into $PR_DIFF and reuse it for the .npmrc, package.json, and package-lock.json extracts instead of calling gh pr diff three times per PR, reducing latency and rate-limit risk. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: nikhil2611 --- .../skills/validate-dependabot-pr/SKILL.md | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/.github/skills/validate-dependabot-pr/SKILL.md b/.github/skills/validate-dependabot-pr/SKILL.md index fe29dbc..1e1a611 100644 --- a/.github/skills/validate-dependabot-pr/SKILL.md +++ b/.github/skills/validate-dependabot-pr/SKILL.md @@ -11,11 +11,20 @@ post PR comments unless the user explicitly asks you to after seeing the report. This repo uses **npm only** (no pnpm-workspace.yaml or yarn.lock). Its HAR baseline lives in the root `.npmrc`: ``` +# REQUIRED - DO NOT REMOVE OR CIRCUMVENT +# HAR (Harness Artifact Registry) configuration for chef org registry=https://pkg.harness.io/pkg/ct8onj8YTdaXtKaFsYCRLg/org-chef-npm/npm/ @jsr:registry=https://pkg.harness.io/pkg/ct8onj8YTdaXtKaFsYCRLg/org-chef-npm/npm/ + +# Block risky lifecycle scripts by default ignore-scripts=true -min-release-age=14 # stricter than the HAR doc's 7-day default — treat 14 as - # this repo's authoritative floor; never suggest lowering it. + +# npm safety gate (days; 14-day cooldown for supply-chain attack mitigation) +# stricter than the HAR doc's 7-day default — treat 14 as this repo's +# authoritative floor; never suggest lowering it. +min-release-age=14 + +# Ensure lockfile is enabled even if a developer has package-lock=false globally package-lock=true ``` @@ -43,19 +52,22 @@ path or host ever changes: ```bash HAR_HOST="$(grep '^registry=' .npmrc | sed -E 's#^registry=https?://([^/]+)/.*#\1#')" if [ -z "$HAR_HOST" ]; then echo "ERROR: could not derive HAR_HOST from .npmrc" >&2; exit 1; fi -gh pr diff --repo chef/vscode-chef | sed -n '/^diff --git a\/\.npmrc /,/^diff --git /p' -gh pr diff --repo chef/vscode-chef | sed -n '/^diff --git a\/package\.json /,/^diff --git /p' -gh pr diff --repo chef/vscode-chef | sed -n '/^diff --git a\/package-lock\.json /,/^diff --git /p' | grep '"resolved"' | grep '^+' | grep -F -v -- "$HAR_HOST" || true +PR_DIFF="$(gh pr diff --repo chef/vscode-chef)" +printf '%s\n' "$PR_DIFF" | sed -n '/^diff --git a\/\.npmrc /,/^diff --git /p' +printf '%s\n' "$PR_DIFF" | sed -n '/^diff --git a\/package\.json /,/^diff --git /p' +printf '%s\n' "$PR_DIFF" | sed -n '/^diff --git a\/package-lock\.json /,/^diff --git /p' | grep '"resolved"' | grep '^+' | grep -F -v -- "$HAR_HOST" || true ``` -Use `sed` ranges (not `grep -A `) so the full `.npmrc`/`package.json` -diff hunks are captured regardless of how many lines they span, and scope -the resolved-URL bypass check to the `package-lock.json` diff section -specifically (that's the only file where `"resolved"` lines matter). Append -`|| true` to the last pipeline — under `set -e` (or when this snippet's -exit status is checked), a `grep -v` that matches nothing exits non-zero, -and "no non-HAR resolved lines found" must be treated as a clean pass, not -a script failure. +Fetch the diff once into `PR_DIFF` and reuse it for all three extracts — +calling `gh pr diff` three times per PR is slow and needlessly increases +the chance of hitting GitHub API rate limits. Use `sed` ranges (not +`grep -A `) so the full `.npmrc`/`package.json` diff hunks are captured +regardless of how many lines they span, and scope the resolved-URL bypass +check to the `package-lock.json` diff section specifically (that's the +only file where `"resolved"` lines matter). Append `|| true` to the last +pipeline — under `set -e` (or when this snippet's exit status is checked), +a `grep -v` that matches nothing exits non-zero, and "no non-HAR resolved +lines found" must be treated as a clean pass, not a script failure. - **Never skip this check if `HAR_HOST` is empty** — an empty pattern passed to `grep -v` would suppress all output and make the bypass check falsely appear From ce19ba37ef6f895ec1b5d7bd955636c569f1d985 Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Tue, 8 Sep 2026 11:12:41 +0530 Subject: [PATCH 10/12] Harden HAR resolved-URL check against lookalike-domain false negatives - Match the full registry origin plus trailing slash ($HAR_ORIGIN/) instead of a bare host substring ($HAR_HOST), so a domain like pkg.harness.io.attacker.example can no longer slip past the check. - Clarify that the local packaging step adds --out for a deterministic artifact path, which the CI build job itself does not pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: nikhil2611 --- .../skills/validate-dependabot-pr/SKILL.md | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/.github/skills/validate-dependabot-pr/SKILL.md b/.github/skills/validate-dependabot-pr/SKILL.md index 1e1a611..7294aa5 100644 --- a/.github/skills/validate-dependabot-pr/SKILL.md +++ b/.github/skills/validate-dependabot-pr/SKILL.md @@ -52,10 +52,12 @@ path or host ever changes: ```bash HAR_HOST="$(grep '^registry=' .npmrc | sed -E 's#^registry=https?://([^/]+)/.*#\1#')" if [ -z "$HAR_HOST" ]; then echo "ERROR: could not derive HAR_HOST from .npmrc" >&2; exit 1; fi +HAR_ORIGIN="$(grep '^registry=' .npmrc | sed -E 's#^registry=(https?://[^/]+)/.*#\1#')" +if [ -z "$HAR_ORIGIN" ]; then echo "ERROR: could not derive HAR_ORIGIN from .npmrc" >&2; exit 1; fi PR_DIFF="$(gh pr diff --repo chef/vscode-chef)" printf '%s\n' "$PR_DIFF" | sed -n '/^diff --git a\/\.npmrc /,/^diff --git /p' printf '%s\n' "$PR_DIFF" | sed -n '/^diff --git a\/package\.json /,/^diff --git /p' -printf '%s\n' "$PR_DIFF" | sed -n '/^diff --git a\/package-lock\.json /,/^diff --git /p' | grep '"resolved"' | grep '^+' | grep -F -v -- "$HAR_HOST" || true +printf '%s\n' "$PR_DIFF" | sed -n '/^diff --git a\/package-lock\.json /,/^diff --git /p' | grep '"resolved"' | grep '^+' | grep -F -v -- "$HAR_ORIGIN/" || true ``` Fetch the diff once into `PR_DIFF` and reuse it for all three extracts — @@ -69,17 +71,24 @@ pipeline — under `set -e` (or when this snippet's exit status is checked), a `grep -v` that matches nothing exits non-zero, and "no non-HAR resolved lines found" must be treated as a clean pass, not a script failure. -- **Never skip this check if `HAR_HOST` is empty** — an empty pattern passed to - `grep -v` would suppress all output and make the bypass check falsely appear - clean. The `exit 1` guard above prevents this; if it fires, stop and report - that HAR compliance could not be verified rather than assuming a pass. +The bypass check matches `$HAR_ORIGIN/` (the full registry origin plus a +trailing slash), not just `$HAR_HOST`, so a lookalike domain that merely +contains the host as a substring (e.g. `pkg.harness.io.attacker.example`) +is correctly flagged instead of slipping past as a false-negative match. +`HAR_HOST` alone is still used for the `.npmrc`-hardening prose below. + +- **Never skip this check if `HAR_HOST` or `HAR_ORIGIN` is empty** — an empty + pattern passed to `grep -v` would suppress all output and make the bypass + check falsely appear clean. The `exit 1` guards above prevent this; if + either fires, stop and report that HAR compliance could not be verified + rather than assuming a pass. - FAIL if the `.npmrc` hunk removes or weakens any of: the `$HAR_HOST` registry URL, `@jsr:registry`, `ignore-scripts=true`, `min-release-age=14` (raising it above 14 is fine; lowering below 14 is not), or `package-lock=true`. - FAIL if the third command above prints any added (`+`) `"resolved"` line - not pointing at `$HAR_HOST` — that means the lockfile was regenerated - bypassing HAR (resolved against `registry.npmjs.org`). + not pointing at `$HAR_ORIGIN/` — that means the lockfile was regenerated + bypassing HAR (resolved against `registry.npmjs.org` or a lookalike host). - PASS if only version/integrity/resolved-HAR-URL fields changed and the hardening lines above are untouched (this is the normal, expected case for routine Dependabot bumps). @@ -146,7 +155,8 @@ idempotent: a lingering worktree directory or an already-checked-out cause `git fetch` to refuse updating a checked-out branch, or `git worktree add` to fail on an existing directory. -Run the same steps as the `build` CI job: +Run the same install + package steps as the `build` CI job (adding `--out` +so the artifact has a deterministic path — CI itself doesn't pass `--out`): ```bash npm ci From 1e858c6ff800a094325b6a7ba94b69d772c575a2 Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Tue, 8 Sep 2026 11:24:36 +0530 Subject: [PATCH 11/12] Replace ambiguous sed range with awk block extractor for HAR diff parsing The prior sed -n '/start/,/end/p' range used '^diff --git ' as the end address, which also matches the start line itself (since every target start pattern like '^diff --git a/package.json ' is a stricter form of '^diff --git '). Same-line start/end address matching is handled inconsistently across sed implementations and can degenerate into printing only the header line, producing a false-negative HAR bypass check. Replace it with an explicit awk helper that only stops printing when a *subsequent* line begins a new diff --git header, which is unambiguous regardless of sed/awk dialect and was verified against a real PR diff. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: nikhil2611 --- .../skills/validate-dependabot-pr/SKILL.md | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/.github/skills/validate-dependabot-pr/SKILL.md b/.github/skills/validate-dependabot-pr/SKILL.md index 7294aa5..64231d2 100644 --- a/.github/skills/validate-dependabot-pr/SKILL.md +++ b/.github/skills/validate-dependabot-pr/SKILL.md @@ -55,18 +55,25 @@ if [ -z "$HAR_HOST" ]; then echo "ERROR: could not derive HAR_HOST from .npmrc" HAR_ORIGIN="$(grep '^registry=' .npmrc | sed -E 's#^registry=(https?://[^/]+)/.*#\1#')" if [ -z "$HAR_ORIGIN" ]; then echo "ERROR: could not derive HAR_ORIGIN from .npmrc" >&2; exit 1; fi PR_DIFF="$(gh pr diff --repo chef/vscode-chef)" -printf '%s\n' "$PR_DIFF" | sed -n '/^diff --git a\/\.npmrc /,/^diff --git /p' -printf '%s\n' "$PR_DIFF" | sed -n '/^diff --git a\/package\.json /,/^diff --git /p' -printf '%s\n' "$PR_DIFF" | sed -n '/^diff --git a\/package-lock\.json /,/^diff --git /p' | grep '"resolved"' | grep '^+' | grep -F -v -- "$HAR_ORIGIN/" || true +extract_diff_block() { awk -v pat="^diff --git a/$1 " '$0 ~ pat {p=1; print; next} /^diff --git / {p=0} p'; } +printf '%s\n' "$PR_DIFF" | extract_diff_block '\.npmrc' +printf '%s\n' "$PR_DIFF" | extract_diff_block 'package\.json' +printf '%s\n' "$PR_DIFF" | extract_diff_block 'package-lock\.json' | grep '"resolved"' | grep '^+' | grep -F -v -- "$HAR_ORIGIN/" || true ``` Fetch the diff once into `PR_DIFF` and reuse it for all three extracts — calling `gh pr diff` three times per PR is slow and needlessly increases -the chance of hitting GitHub API rate limits. Use `sed` ranges (not -`grep -A `) so the full `.npmrc`/`package.json` diff hunks are captured -regardless of how many lines they span, and scope the resolved-URL bypass -check to the `package-lock.json` diff section specifically (that's the -only file where `"resolved"` lines matter). Append `|| true` to the last +the chance of hitting GitHub API rate limits. Use the `extract_diff_block` +awk helper (not a `sed '/start/,/end/p'` range or `grep -A `) so the +full `.npmrc`/`package.json` diff hunks are captured regardless of how many +lines they span: the helper turns off printing only when a *later* line +starts a new `diff --git` header, so it can't be tripped up by the opening +header line itself also matching that same broad end pattern — a real risk +with a `sed` range whose start and end addresses can both match on line +one, where behavior differs across sed implementations and can otherwise +degenerate into printing just the header. Scope the resolved-URL bypass +check to the `package-lock.json` diff block specifically (that's the only +file where `"resolved"` lines matter). Append `|| true` to the last pipeline — under `set -e` (or when this snippet's exit status is checked), a `grep -v` that matches nothing exits non-zero, and "no non-HAR resolved lines found" must be treated as a clean pass, not a script failure. From 0962f8a987f3492945bc2bbc647571be0114c3b5 Mon Sep 17 00:00:00 2001 From: nikhil2611 Date: Tue, 8 Sep 2026 11:29:29 +0530 Subject: [PATCH 12/12] Gate local npm ci/packaging on HAR compliance passing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Explicitly skip the local npm ci / vsce package / install steps when the HAR compliance check (a) fails or can't be verified (e.g. a weakened .npmrc, a non-HAR-resolved lockfile entry, or the HAR_HOST/HAR_ORIGIN empty-guard tripping). Previously the skill only gated local packaging on CI status, so a PR that failed HAR compliance but still had green CI could still run npm ci locally — risking lifecycle script execution or non-HAR dependency resolution. Report these as 'not run — HAR compliance failed/unverifiable' in the final table, mirroring the existing CI-red skip convention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: nikhil2611 --- .github/skills/validate-dependabot-pr/SKILL.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.github/skills/validate-dependabot-pr/SKILL.md b/.github/skills/validate-dependabot-pr/SKILL.md index 64231d2..fa568c1 100644 --- a/.github/skills/validate-dependabot-pr/SKILL.md +++ b/.github/skills/validate-dependabot-pr/SKILL.md @@ -134,6 +134,17 @@ instead, use `gh pr view --repo chef/vscode-chef --json statusCheckRollup` ### d. Packaging verification +**Gate this step on HAR compliance (check a).** Only run the local `npm ci` / +`vsce package` steps below if check (a) PASSED — i.e. `.npmrc` hardening was +untouched (or only strengthened) and no non-HAR `"resolved"` lines were +found. If check (a) FAILED, or HAR compliance could not be verified (for +example the `HAR_HOST`/`HAR_ORIGIN` guard tripped), skip packaging and +installation (section e) entirely and report both as "not run — HAR +compliance failed/unverifiable" in the final table. Running `npm ci` against +a weakened `.npmrc` (e.g. `ignore-scripts` removed) or a lockfile resolving +outside HAR risks executing untrusted lifecycle scripts or pulling packages +from a non-HAR registry — never do this locally even to "confirm" a failure. + Check out the PR into an isolated git worktree so the user's current branch/working tree is never disturbed. Capture the original repo directory first so cleanup can reliably return to it (don't hardcode @@ -225,5 +236,10 @@ Print one summary table across all open Dependabot PRs, one row per PR: time re-deriving a failure GitHub already reported. Still run the packaging and installation steps locally whenever CI is green, or whenever the user explicitly asks you to double-check a CI failure yourself. +- If the HAR compliance check (a) fails or can't be verified, always skip + packaging and installation for that PR regardless of CI status (note both + as "not run — HAR compliance failed/unverifiable") — never run `npm ci` or + `vsce package` locally against a PR with weakened `.npmrc` hardening or a + non-HAR-resolved lockfile. - Do not approve, comment on, or merge any PR. End the report by reminding the user that merging is manual, per team policy.