From cffcf05e7a8ce89e2defce581f6e1536350fc501 Mon Sep 17 00:00:00 2001
From: chenhz01 <chenhz01@users.noreply.github.com>
Date: Sun, 11 Oct 2026 18:52:44 +0800
Subject: [PATCH] =?UTF-8?q?hooks:=20sed=20labels=20end=20at=20semicolon=20?=
 =?UTF-8?q?=E2=80=94=20commands=20after=20b/t/T/:=20are=20still=20scanned?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CodeRabbit security finding on the merged sed work (CWE-693, verified live
on main via #3495): GNU sed labels for b, t, T and : end at ';' or newline —
verified against GNU sed 4.9 ('bar;e cmd' parses label 'a' and still runs
cmd; '}' is not a label terminator). Reading those operands to end of line
let 'sed :a;e git commit --no-verify' pass the hook while sed executed the
e command.

Labels are now read up to ';' or newline and command parsing resumes after
them. Text, filename and quit operands (a, c, i, r, w, q, Q) keep the
end-of-line reading: nothing after them on the line executes.

5 regression tests pin the three bypass paths and the benign boundary
(a bare label and a bare branch stay data). Suite: 817/817.
---
 scripts/hooks/block-no-verify.js    | 14 +++++++++++++-
 tests/hooks/block-no-verify.test.js | 11 ++++++++++-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/scripts/hooks/block-no-verify.js b/scripts/hooks/block-no-verify.js
index 1d19e5c35..6774ae75f 100644
--- a/scripts/hooks/block-no-verify.js
+++ b/scripts/hooks/block-no-verify.js
@@ -586,6 +586,17 @@ function readSedLineOperand(script, i, budget) {
   return { end, parts: [], stdinExec: false };
 }
 
+// Branch and label commands (b, t, T, :) take a label that ends at `;` or
+// newline — verified against GNU sed 4.9: ':a;e cmd' and 'ba;e cmd' both
+// parse the label up to `;` and still run the following `e`. Commands after
+// the label therefore continue to be scanned, so return the label end and
+// let the caller resume command parsing there.
+function readSedLabel(script, i, budget) {
+  let end = i + 1;
+  while (end < script.length && script[end] !== ';' && script[end] !== '\n') { budget.spend(); end++; }
+  return { end, parts: [], stdinExec: false };
+}
+
 function readSedCommand(script, i, budget) {
   const letter = script[i];
   if (letter === 's') {
@@ -602,7 +613,8 @@ function readSedCommand(script, i, budget) {
     return sub ? { end: sub.end, parts: [], stdinExec: false } : null;
   }
   if (letter === 'e') return readSedExecute(script, i, budget);
-  if ('acirwWb:tTqQ'.includes(letter)) return readSedLineOperand(script, i, budget);
+  if ('btT:'.includes(letter)) return readSedLabel(script, i, budget);
+  if ('acirwWqQ'.includes(letter)) return readSedLineOperand(script, i, budget);
   if ('dDgGhHlnNpPxFzv='.includes(letter)) return { end: i + 1, parts: [], stdinExec: false };
   return null; // unmodeled command letter: keep the opaque default
 }
diff --git a/tests/hooks/block-no-verify.test.js b/tests/hooks/block-no-verify.test.js
index 56549e721..bf456d050 100644
--- a/tests/hooks/block-no-verify.test.js
+++ b/tests/hooks/block-no-verify.test.js
@@ -2183,7 +2183,16 @@ const sedCases = [
   ['unknown escape decoding bypass is conservatively blocked', "printf x | sed 's/x/\\git commit --no-verify/e'", 2],
   ['escaped-newline continued e operand is conservatively blocked', "printf x | sed -e 'e git commit \\\n--no-verify'", 2],
   ['octal escape decoding bypass is conservatively blocked', "printf x | sed 's/x/\\147it commit --no-verify/e'", 2],
-  ['backslash in a non-executing script stays data', "sed 's/a\\/b/c/' 'git commit --no-verify'", 0]
+  ['backslash in a non-executing script stays data', "sed 's/a\\/b/c/' 'git commit --no-verify'", 0],
+  // Labels end at `;` and following commands still execute (GNU sed 4.9
+  // verified: ':a;e cmd' runs cmd). Reading label operands to end of line
+  // let an e command hide behind a label (CodeRabbit finding, landed via
+  // #3495) — the label reader must stop at `;` so the rest is scanned.
+  ['e command after a label is scanned', "sed ':a;e git commit --no-verify'", 2],
+  ['e command after a b branch is scanned', "printf x | sed 'ba;e git commit --no-verify'", 2],
+  ['e command after a t branch is scanned', "printf x | sed 'tX;e git commit --no-verify'", 2],
+  ['plain label without following commands stays data', "sed ':a' 'git commit --no-verify'", 0],
+  ['b branch alone stays data', "sed 'bx' 'git commit --no-verify'", 0]
 ];
 for (const [name, command, expected] of sedCases) {
   if (test(`sed model ${name}`, () => {