)]}'
{
  "commit": "de1d60db5f5f057fcf772584dff9a2f1940b45ec",
  "tree": "77ba78c66242596bb2389de278e4356baeac4b61",
  "parents": [
    "705db9bc77cf9bf8f82df76ac22338f6e745b4fb"
  ],
  "author": {
    "name": "BarakSrour",
    "email": "85163444+BarakSrour@users.noreply.github.com",
    "time": "Mon Sep 07 06:35:31 2026 -0400"
  },
  "committer": {
    "name": "GitHub",
    "email": "noreply@github.com",
    "time": "Mon Sep 07 10:35:31 2026 +0000"
  },
  "message": "bcr-pr-reviewer: validate module version directories before diffing (#2842)\n\n## Summary\n\n`runDiffModule()` interpolates version strings directly into the paths\nit hands to `diff`:\n\n```js\nconst previousVersion \u003d metadata.versions[versionIndex - 1];\nconst diffArgs \u003d [\u0027--color\u003dalways\u0027, \u0027-urN\u0027,\n                  `modules/${moduleName}/${previousVersion}`,\n                  `modules/${moduleName}/${versionName}`];\n```\n\n`previousVersion` is read out of the module\u0027s `metadata.json` at\n`refs/pull/${prNumber}/head`, so its value comes from the PR being\nreviewed and is chosen by whoever opened that PR. A `versions` entry\nsuch as `\"../../..\"` makes `diff` descend outside the module directory,\nand `diff -urN` prints the contents of the files it finds to stdout,\nwhich the action logs.\n\n`generate_module_diff.yml` in `bazel-central-registry` runs this on\n`pull_request` for any PR touching `modules/**`, so reaching it needs\nonly an ordinary PR.\n\n## Impact\n\nLimited, and deliberately stated as such. That workflow uses the default\n`github.token` on a `pull_request` trigger, so a fork PR gets a\nread-only token and no repository secrets. What the bug gives is a read\nof runner-local files into a public workflow log, not code execution and\nnot access to anything the PR author does not already have.\n\nThis is the same input, in the same workflow, that #2220 handled by\nreplacing `execSync` with `spawnSync`. That change removed the shell, so\nthe string can no longer be interpreted as a command, but it is still\nused as a path. #2220 was filed as an ordinary PR on the reasoning that\nthis workflow\u0027s permissions are limited enough not to warrant a private\nreport, and this is strictly the milder of the two, so I have followed\nthe same route. Happy to move it if you would rather it went to the\nsecurity team.\n\n## Fix\n\nAdd `moduleVersionDir(moduleName, version)`, which resolves the\ncandidate directory and returns `null` unless it sits directly inside\n`modules/\u003cmoduleName\u003e/`, and skip a module whose version fails that\ntest.\n\nThe check is a containment test on the resolved path rather than a\npattern match on the version string, so it does not need to know the\nBazel version grammar and does not reject versions containing dots,\ndashes or plus signs. Output for legitimate input is byte-for-byte\nunchanged.\n\n## Test plan\n\n- `node -c index.js`: parses cleanly.\n- `npm test`: 7/7 pass (the 3 existing `getPrApprovers` tests plus 4 new\nones covering `../../..`, `..`, `../other_module/1.0.0`, `nested/1.0.0`,\n`/etc`, `/etc/passwd`, `.` and `\"\"`, and asserting that `0.9.0-rc.1..2`\nand `1.0.0+build.5` are still accepted).\n- Ran the unmodified action end to end against a stubbed GitHub API and\na fabricated PR, in a directory tree laid out like the runner\u0027s, in\nthree arms:\n- legitimate `previousVersion` of `0.9.0`: `diff --color\u003dalways -urN\nmodules/testmod/0.9.0 modules/testmod/1.0.0`, normal output;\n- `previousVersion` of `../../..`: `diff --color\u003dalways -urN\nmodules/testmod/../../.. modules/testmod/1.0.0`, and the contents of a\nfile placed outside the checkout appear in the action\u0027s output;\n- `previousVersion` of `0.9.0-rc.1..2`: stays inside the module\ndirectory, confirming that the dots are not what matters.\n- Re-ran the identical three arms against the patched file: arm 1 output\nunchanged, arm 3 unchanged, arm 2 now stops at `Refusing to diff module\ntestmod: a version does not name a directory inside modules/testmod/`\nwith no file contents in the log.",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "5d04d59532122a1332593b6e20acc613a29a56fe",
      "old_mode": 33188,
      "old_path": "actions/bcr-pr-reviewer/index.js",
      "new_id": "776b2ee6583db69745876af5fc2cf890eef343fc",
      "new_mode": 33188,
      "new_path": "actions/bcr-pr-reviewer/index.js"
    },
    {
      "type": "modify",
      "old_id": "21332d9424486e5b2f540e38c3a70bf7fda892b2",
      "old_mode": 33188,
      "old_path": "actions/bcr-pr-reviewer/index.test.js",
      "new_id": "d6e6e478dbf00fef8fe1550ee837d7bd9e31c263",
      "new_mode": 33188,
      "new_path": "actions/bcr-pr-reviewer/index.test.js"
    }
  ]
}