fix(security): prevent sed injection and integer overflow (fixes #2961, #2962) - #2964

Merged
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962
Mar 24, 2026
Merged

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962)#2964
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: [HIGH] Two security vulnerabilities: (1) Base64 validation bypass in qa.sh safe_substitute allows sed delimiter injection; (2) SPAWN_ISSUE integer validation may not prevent leading zeros/overflow enabling path traversal.

Changes

Fix#1: safe_substitute sed delimiter injection (#2961)

All 4 scripts (qa.sh, refactor.sh, security.sh, discovery.sh):

  • Switch sed delimiter from | to \x01 (SOH control character) which cannot appear in normal text input
  • Remove now-unnecessary | escaping from the value escaping pipeline
  • Add explicit rejection of values containing \x01 as defense-in-depth
  • Keep existing \ and & escaping (required sed metacharacter escaping)

Fix#2: SPAWN_ISSUE integer validation (#2962)

qa.sh:

  • Fix regex from ^[0-9]+$ to ^[1-9][0-9]*$ to reject leading zeros and zero
  • Add 32-bit signed integer range check (max 2,147,483,647 matching GitHub's limit)

refactor.sh, security.sh:

  • Already had ^[1-9][0-9]*$ regex, now also have range validation added

Testing

  • bash -n passes on all 4 modified scripts
  • Functional testing confirms:
    • Pipe characters in values work correctly (previously the delimiter)
    • Backslashes and ampersands are properly escaped
    • Path values substitute correctly
    • \x01 in values is rejected

Fixes#2961
Fixes#2962

-- refactor/security-auditor

… validation
safe_substitute: Switch sed delimiter from | to \x01 (SOH control char) across
qa.sh, refactor.sh, security.sh, and discovery.sh. This eliminates delimiter
injection regardless of value content, since \x01 cannot appear in normal input.
Values containing \x01 are explicitly rejected as defense-in-depth.
SPAWN_ISSUE: Fix qa.sh validation from ^[0-9]+$ to ^[1-9][0-9]*$ to reject
leading zeros and zero itself. Add 32-bit signed integer range check
(max 2147483647) to all three scripts (qa.sh, refactor.sh, security.sh)
to prevent integer overflow in downstream consumers.
Fixes#2961Fixes#2962
Agent: security-auditor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@louisgvlouisgv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: ba294fa

Summary

This PR addresses two HIGH-severity security vulnerabilities in the autonomous agent orchestration scripts. The fixes are well-designed and implement defense-in-depth strategies.

Findings

NONE - All security issues properly addressed:

  1. sed Delimiter Injection (Issue #2961) - FIXED

    • Root Cause: Using pipe character (|) as sed delimiter allowed malicious values containing | to inject arbitrary sed commands
    • Fix: Switch to \x01 (SOH control character) as delimiter, which cannot appear in normal text
    • Defense-in-depth: Added explicit rejection of values containing \x01
    • Impact: All 4 scripts (discovery.sh, qa.sh, refactor.sh, security.sh) consistently patched
  2. Integer Overflow in SPAWN_ISSUE (Issue #2962) - FIXED

    • Root Cause: qa.sh allowed leading zeros (e.g., "042") and didn't enforce GitHub's 32-bit signed int max (2,147,483,647)
    • Fix:
      • Regex updated from ^[0-9]+$ to ^[1-9][0-9]*$ (rejects leading zeros and zero itself)
      • Added length check (max 10 digits) and numeric range check (≤ 2147483647)
    • Consistency: refactor.sh and security.sh already had correct regex, now also have range validation
    • Impact: Prevents path traversal attacks via issue number manipulation

Code Quality

  • No regressions: Removed unnecessary pipe character escaping (no longer needed with \x01 delimiter)
  • Consistent implementation: All 4 scripts use identical safe_substitute() logic
  • Clear documentation: Comments explain the security rationale
  • macOS compatible: grep -qP requires GNU grep (available via brew on macOS, standard on Linux VMs)

Tests

✅ bash -n: PASS on all 4 modified scripts
❌ bun test: N/A (no TypeScript changes)
✅ curl|bash: OK (scripts source from GitHub URLs, no relative paths)
✅ macOS compat: OK (printf instead of echo -e, no bash 4+ features)

Threat Model Assessment

Before: An attacker controlling SPAWN_ISSUE or values passed to safe_substitute could:

  • Execute arbitrary sed commands via delimiter injection
  • Traverse filesystem paths via leading-zero issue numbers

After: Both attack vectors eliminated:

  • \x01 delimiter cannot appear in text input
  • SPAWN_ISSUE strictly validated as 1-2147483647

Residual Risk: None identified

Recommendations

✅ This PR should be merged immediately to remediate HIGH-severity vulnerabilities in production automation.


-- security/pr-reviewer

@louisgvlouisgv added the security-approved Security review approved label Mar 24, 2026
@louisgv
louisgv merged commit e045cf6 into mainMar 24, 2026
6 checks passed
@louisgv
louisgv deleted the fix/security-2961-2962 branch March 24, 2026 18:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-approvedSecurity review approved

Projects

None yet

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962) - #2964

Merged
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962
Mar 24, 2026
Merged

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962)#2964
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: [HIGH] Two security vulnerabilities: (1) Base64 validation bypass in qa.sh safe_substitute allows sed delimiter injection; (2) SPAWN_ISSUE integer validation may not prevent leading zeros/overflow enabling path traversal.

Changes

Fix#1: safe_substitute sed delimiter injection (#2961)

All 4 scripts (qa.sh, refactor.sh, security.sh, discovery.sh):

  • Switch sed delimiter from | to \x01 (SOH control character) which cannot appear in normal text input
  • Remove now-unnecessary | escaping from the value escaping pipeline
  • Add explicit rejection of values containing \x01 as defense-in-depth
  • Keep existing \ and & escaping (required sed metacharacter escaping)

Fix#2: SPAWN_ISSUE integer validation (#2962)

qa.sh:

  • Fix regex from ^[0-9]+$ to ^[1-9][0-9]*$ to reject leading zeros and zero
  • Add 32-bit signed integer range check (max 2,147,483,647 matching GitHub's limit)

refactor.sh, security.sh:

  • Already had ^[1-9][0-9]*$ regex, now also have range validation added

Testing

  • bash -n passes on all 4 modified scripts
  • Functional testing confirms:
    • Pipe characters in values work correctly (previously the delimiter)
    • Backslashes and ampersands are properly escaped
    • Path values substitute correctly
    • \x01 in values is rejected

Fixes#2961
Fixes#2962

-- refactor/security-auditor

… validation
safe_substitute: Switch sed delimiter from | to \x01 (SOH control char) across
qa.sh, refactor.sh, security.sh, and discovery.sh. This eliminates delimiter
injection regardless of value content, since \x01 cannot appear in normal input.
Values containing \x01 are explicitly rejected as defense-in-depth.
SPAWN_ISSUE: Fix qa.sh validation from ^[0-9]+$ to ^[1-9][0-9]*$ to reject
leading zeros and zero itself. Add 32-bit signed integer range check
(max 2147483647) to all three scripts (qa.sh, refactor.sh, security.sh)
to prevent integer overflow in downstream consumers.
Fixes#2961Fixes#2962
Agent: security-auditor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@louisgvlouisgv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: ba294fa

Summary

This PR addresses two HIGH-severity security vulnerabilities in the autonomous agent orchestration scripts. The fixes are well-designed and implement defense-in-depth strategies.

Findings

NONE - All security issues properly addressed:

  1. sed Delimiter Injection (Issue #2961) - FIXED

    • Root Cause: Using pipe character (|) as sed delimiter allowed malicious values containing | to inject arbitrary sed commands
    • Fix: Switch to \x01 (SOH control character) as delimiter, which cannot appear in normal text
    • Defense-in-depth: Added explicit rejection of values containing \x01
    • Impact: All 4 scripts (discovery.sh, qa.sh, refactor.sh, security.sh) consistently patched
  2. Integer Overflow in SPAWN_ISSUE (Issue #2962) - FIXED

    • Root Cause: qa.sh allowed leading zeros (e.g., "042") and didn't enforce GitHub's 32-bit signed int max (2,147,483,647)
    • Fix:
      • Regex updated from ^[0-9]+$ to ^[1-9][0-9]*$ (rejects leading zeros and zero itself)
      • Added length check (max 10 digits) and numeric range check (≤ 2147483647)
    • Consistency: refactor.sh and security.sh already had correct regex, now also have range validation
    • Impact: Prevents path traversal attacks via issue number manipulation

Code Quality

  • No regressions: Removed unnecessary pipe character escaping (no longer needed with \x01 delimiter)
  • Consistent implementation: All 4 scripts use identical safe_substitute() logic
  • Clear documentation: Comments explain the security rationale
  • macOS compatible: grep -qP requires GNU grep (available via brew on macOS, standard on Linux VMs)

Tests

✅ bash -n: PASS on all 4 modified scripts
❌ bun test: N/A (no TypeScript changes)
✅ curl|bash: OK (scripts source from GitHub URLs, no relative paths)
✅ macOS compat: OK (printf instead of echo -e, no bash 4+ features)

Threat Model Assessment

Before: An attacker controlling SPAWN_ISSUE or values passed to safe_substitute could:

  • Execute arbitrary sed commands via delimiter injection
  • Traverse filesystem paths via leading-zero issue numbers

After: Both attack vectors eliminated:

  • \x01 delimiter cannot appear in text input
  • SPAWN_ISSUE strictly validated as 1-2147483647

Residual Risk: None identified

Recommendations

✅ This PR should be merged immediately to remediate HIGH-severity vulnerabilities in production automation.


-- security/pr-reviewer

@louisgvlouisgv added the security-approved Security review approved label Mar 24, 2026
@louisgv
louisgv merged commit e045cf6 into mainMar 24, 2026
6 checks passed
@louisgv
louisgv deleted the fix/security-2961-2962 branch March 24, 2026 18:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-approvedSecurity review approved

Projects

None yet

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962) - #2964

Merged
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962
Mar 24, 2026
Merged

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962)#2964
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: [HIGH] Two security vulnerabilities: (1) Base64 validation bypass in qa.sh safe_substitute allows sed delimiter injection; (2) SPAWN_ISSUE integer validation may not prevent leading zeros/overflow enabling path traversal.

Changes

Fix#1: safe_substitute sed delimiter injection (#2961)

All 4 scripts (qa.sh, refactor.sh, security.sh, discovery.sh):

  • Switch sed delimiter from | to \x01 (SOH control character) which cannot appear in normal text input
  • Remove now-unnecessary | escaping from the value escaping pipeline
  • Add explicit rejection of values containing \x01 as defense-in-depth
  • Keep existing \ and & escaping (required sed metacharacter escaping)

Fix#2: SPAWN_ISSUE integer validation (#2962)

qa.sh:

  • Fix regex from ^[0-9]+$ to ^[1-9][0-9]*$ to reject leading zeros and zero
  • Add 32-bit signed integer range check (max 2,147,483,647 matching GitHub's limit)

refactor.sh, security.sh:

  • Already had ^[1-9][0-9]*$ regex, now also have range validation added

Testing

  • bash -n passes on all 4 modified scripts
  • Functional testing confirms:
    • Pipe characters in values work correctly (previously the delimiter)
    • Backslashes and ampersands are properly escaped
    • Path values substitute correctly
    • \x01 in values is rejected

Fixes#2961
Fixes#2962

-- refactor/security-auditor

… validation
safe_substitute: Switch sed delimiter from | to \x01 (SOH control char) across
qa.sh, refactor.sh, security.sh, and discovery.sh. This eliminates delimiter
injection regardless of value content, since \x01 cannot appear in normal input.
Values containing \x01 are explicitly rejected as defense-in-depth.
SPAWN_ISSUE: Fix qa.sh validation from ^[0-9]+$ to ^[1-9][0-9]*$ to reject
leading zeros and zero itself. Add 32-bit signed integer range check
(max 2147483647) to all three scripts (qa.sh, refactor.sh, security.sh)
to prevent integer overflow in downstream consumers.
Fixes#2961Fixes#2962
Agent: security-auditor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@louisgvlouisgv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: ba294fa

Summary

This PR addresses two HIGH-severity security vulnerabilities in the autonomous agent orchestration scripts. The fixes are well-designed and implement defense-in-depth strategies.

Findings

NONE - All security issues properly addressed:

  1. sed Delimiter Injection (Issue #2961) - FIXED

    • Root Cause: Using pipe character (|) as sed delimiter allowed malicious values containing | to inject arbitrary sed commands
    • Fix: Switch to \x01 (SOH control character) as delimiter, which cannot appear in normal text
    • Defense-in-depth: Added explicit rejection of values containing \x01
    • Impact: All 4 scripts (discovery.sh, qa.sh, refactor.sh, security.sh) consistently patched
  2. Integer Overflow in SPAWN_ISSUE (Issue #2962) - FIXED

    • Root Cause: qa.sh allowed leading zeros (e.g., "042") and didn't enforce GitHub's 32-bit signed int max (2,147,483,647)
    • Fix:
      • Regex updated from ^[0-9]+$ to ^[1-9][0-9]*$ (rejects leading zeros and zero itself)
      • Added length check (max 10 digits) and numeric range check (≤ 2147483647)
    • Consistency: refactor.sh and security.sh already had correct regex, now also have range validation
    • Impact: Prevents path traversal attacks via issue number manipulation

Code Quality

  • No regressions: Removed unnecessary pipe character escaping (no longer needed with \x01 delimiter)
  • Consistent implementation: All 4 scripts use identical safe_substitute() logic
  • Clear documentation: Comments explain the security rationale
  • macOS compatible: grep -qP requires GNU grep (available via brew on macOS, standard on Linux VMs)

Tests

✅ bash -n: PASS on all 4 modified scripts
❌ bun test: N/A (no TypeScript changes)
✅ curl|bash: OK (scripts source from GitHub URLs, no relative paths)
✅ macOS compat: OK (printf instead of echo -e, no bash 4+ features)

Threat Model Assessment

Before: An attacker controlling SPAWN_ISSUE or values passed to safe_substitute could:

  • Execute arbitrary sed commands via delimiter injection
  • Traverse filesystem paths via leading-zero issue numbers

After: Both attack vectors eliminated:

  • \x01 delimiter cannot appear in text input
  • SPAWN_ISSUE strictly validated as 1-2147483647

Residual Risk: None identified

Recommendations

✅ This PR should be merged immediately to remediate HIGH-severity vulnerabilities in production automation.


-- security/pr-reviewer

@louisgvlouisgv added the security-approved Security review approved label Mar 24, 2026
@louisgv
louisgv merged commit e045cf6 into mainMar 24, 2026
6 checks passed
@louisgv
louisgv deleted the fix/security-2961-2962 branch March 24, 2026 18:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-approvedSecurity review approved

Projects

None yet

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962) - #2964

Merged
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962
Mar 24, 2026
Merged

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962)#2964
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: [HIGH] Two security vulnerabilities: (1) Base64 validation bypass in qa.sh safe_substitute allows sed delimiter injection; (2) SPAWN_ISSUE integer validation may not prevent leading zeros/overflow enabling path traversal.

Changes

Fix#1: safe_substitute sed delimiter injection (#2961)

All 4 scripts (qa.sh, refactor.sh, security.sh, discovery.sh):

  • Switch sed delimiter from | to \x01 (SOH control character) which cannot appear in normal text input
  • Remove now-unnecessary | escaping from the value escaping pipeline
  • Add explicit rejection of values containing \x01 as defense-in-depth
  • Keep existing \ and & escaping (required sed metacharacter escaping)

Fix#2: SPAWN_ISSUE integer validation (#2962)

qa.sh:

  • Fix regex from ^[0-9]+$ to ^[1-9][0-9]*$ to reject leading zeros and zero
  • Add 32-bit signed integer range check (max 2,147,483,647 matching GitHub's limit)

refactor.sh, security.sh:

  • Already had ^[1-9][0-9]*$ regex, now also have range validation added

Testing

  • bash -n passes on all 4 modified scripts
  • Functional testing confirms:
    • Pipe characters in values work correctly (previously the delimiter)
    • Backslashes and ampersands are properly escaped
    • Path values substitute correctly
    • \x01 in values is rejected

Fixes#2961
Fixes#2962

-- refactor/security-auditor

… validation
safe_substitute: Switch sed delimiter from | to \x01 (SOH control char) across
qa.sh, refactor.sh, security.sh, and discovery.sh. This eliminates delimiter
injection regardless of value content, since \x01 cannot appear in normal input.
Values containing \x01 are explicitly rejected as defense-in-depth.
SPAWN_ISSUE: Fix qa.sh validation from ^[0-9]+$ to ^[1-9][0-9]*$ to reject
leading zeros and zero itself. Add 32-bit signed integer range check
(max 2147483647) to all three scripts (qa.sh, refactor.sh, security.sh)
to prevent integer overflow in downstream consumers.
Fixes#2961Fixes#2962
Agent: security-auditor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@louisgvlouisgv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: ba294fa

Summary

This PR addresses two HIGH-severity security vulnerabilities in the autonomous agent orchestration scripts. The fixes are well-designed and implement defense-in-depth strategies.

Findings

NONE - All security issues properly addressed:

  1. sed Delimiter Injection (Issue #2961) - FIXED

    • Root Cause: Using pipe character (|) as sed delimiter allowed malicious values containing | to inject arbitrary sed commands
    • Fix: Switch to \x01 (SOH control character) as delimiter, which cannot appear in normal text
    • Defense-in-depth: Added explicit rejection of values containing \x01
    • Impact: All 4 scripts (discovery.sh, qa.sh, refactor.sh, security.sh) consistently patched
  2. Integer Overflow in SPAWN_ISSUE (Issue #2962) - FIXED

    • Root Cause: qa.sh allowed leading zeros (e.g., "042") and didn't enforce GitHub's 32-bit signed int max (2,147,483,647)
    • Fix:
      • Regex updated from ^[0-9]+$ to ^[1-9][0-9]*$ (rejects leading zeros and zero itself)
      • Added length check (max 10 digits) and numeric range check (≤ 2147483647)
    • Consistency: refactor.sh and security.sh already had correct regex, now also have range validation
    • Impact: Prevents path traversal attacks via issue number manipulation

Code Quality

  • No regressions: Removed unnecessary pipe character escaping (no longer needed with \x01 delimiter)
  • Consistent implementation: All 4 scripts use identical safe_substitute() logic
  • Clear documentation: Comments explain the security rationale
  • macOS compatible: grep -qP requires GNU grep (available via brew on macOS, standard on Linux VMs)

Tests

✅ bash -n: PASS on all 4 modified scripts
❌ bun test: N/A (no TypeScript changes)
✅ curl|bash: OK (scripts source from GitHub URLs, no relative paths)
✅ macOS compat: OK (printf instead of echo -e, no bash 4+ features)

Threat Model Assessment

Before: An attacker controlling SPAWN_ISSUE or values passed to safe_substitute could:

  • Execute arbitrary sed commands via delimiter injection
  • Traverse filesystem paths via leading-zero issue numbers

After: Both attack vectors eliminated:

  • \x01 delimiter cannot appear in text input
  • SPAWN_ISSUE strictly validated as 1-2147483647

Residual Risk: None identified

Recommendations

✅ This PR should be merged immediately to remediate HIGH-severity vulnerabilities in production automation.


-- security/pr-reviewer

@louisgvlouisgv added the security-approved Security review approved label Mar 24, 2026
@louisgv
louisgv merged commit e045cf6 into mainMar 24, 2026
6 checks passed
@louisgv
louisgv deleted the fix/security-2961-2962 branch March 24, 2026 18:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-approvedSecurity review approved

Projects

None yet

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962) - #2964

Merged
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962
Mar 24, 2026
Merged

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962)#2964
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: [HIGH] Two security vulnerabilities: (1) Base64 validation bypass in qa.sh safe_substitute allows sed delimiter injection; (2) SPAWN_ISSUE integer validation may not prevent leading zeros/overflow enabling path traversal.

Changes

Fix#1: safe_substitute sed delimiter injection (#2961)

All 4 scripts (qa.sh, refactor.sh, security.sh, discovery.sh):

  • Switch sed delimiter from | to \x01 (SOH control character) which cannot appear in normal text input
  • Remove now-unnecessary | escaping from the value escaping pipeline
  • Add explicit rejection of values containing \x01 as defense-in-depth
  • Keep existing \ and & escaping (required sed metacharacter escaping)

Fix#2: SPAWN_ISSUE integer validation (#2962)

qa.sh:

  • Fix regex from ^[0-9]+$ to ^[1-9][0-9]*$ to reject leading zeros and zero
  • Add 32-bit signed integer range check (max 2,147,483,647 matching GitHub's limit)

refactor.sh, security.sh:

  • Already had ^[1-9][0-9]*$ regex, now also have range validation added

Testing

  • bash -n passes on all 4 modified scripts
  • Functional testing confirms:
    • Pipe characters in values work correctly (previously the delimiter)
    • Backslashes and ampersands are properly escaped
    • Path values substitute correctly
    • \x01 in values is rejected

Fixes#2961
Fixes#2962

-- refactor/security-auditor

… validation
safe_substitute: Switch sed delimiter from | to \x01 (SOH control char) across
qa.sh, refactor.sh, security.sh, and discovery.sh. This eliminates delimiter
injection regardless of value content, since \x01 cannot appear in normal input.
Values containing \x01 are explicitly rejected as defense-in-depth.
SPAWN_ISSUE: Fix qa.sh validation from ^[0-9]+$ to ^[1-9][0-9]*$ to reject
leading zeros and zero itself. Add 32-bit signed integer range check
(max 2147483647) to all three scripts (qa.sh, refactor.sh, security.sh)
to prevent integer overflow in downstream consumers.
Fixes#2961Fixes#2962
Agent: security-auditor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@louisgvlouisgv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: ba294fa

Summary

This PR addresses two HIGH-severity security vulnerabilities in the autonomous agent orchestration scripts. The fixes are well-designed and implement defense-in-depth strategies.

Findings

NONE - All security issues properly addressed:

  1. sed Delimiter Injection (Issue #2961) - FIXED

    • Root Cause: Using pipe character (|) as sed delimiter allowed malicious values containing | to inject arbitrary sed commands
    • Fix: Switch to \x01 (SOH control character) as delimiter, which cannot appear in normal text
    • Defense-in-depth: Added explicit rejection of values containing \x01
    • Impact: All 4 scripts (discovery.sh, qa.sh, refactor.sh, security.sh) consistently patched
  2. Integer Overflow in SPAWN_ISSUE (Issue #2962) - FIXED

    • Root Cause: qa.sh allowed leading zeros (e.g., "042") and didn't enforce GitHub's 32-bit signed int max (2,147,483,647)
    • Fix:
      • Regex updated from ^[0-9]+$ to ^[1-9][0-9]*$ (rejects leading zeros and zero itself)
      • Added length check (max 10 digits) and numeric range check (≤ 2147483647)
    • Consistency: refactor.sh and security.sh already had correct regex, now also have range validation
    • Impact: Prevents path traversal attacks via issue number manipulation

Code Quality

  • No regressions: Removed unnecessary pipe character escaping (no longer needed with \x01 delimiter)
  • Consistent implementation: All 4 scripts use identical safe_substitute() logic
  • Clear documentation: Comments explain the security rationale
  • macOS compatible: grep -qP requires GNU grep (available via brew on macOS, standard on Linux VMs)

Tests

✅ bash -n: PASS on all 4 modified scripts
❌ bun test: N/A (no TypeScript changes)
✅ curl|bash: OK (scripts source from GitHub URLs, no relative paths)
✅ macOS compat: OK (printf instead of echo -e, no bash 4+ features)

Threat Model Assessment

Before: An attacker controlling SPAWN_ISSUE or values passed to safe_substitute could:

  • Execute arbitrary sed commands via delimiter injection
  • Traverse filesystem paths via leading-zero issue numbers

After: Both attack vectors eliminated:

  • \x01 delimiter cannot appear in text input
  • SPAWN_ISSUE strictly validated as 1-2147483647

Residual Risk: None identified

Recommendations

✅ This PR should be merged immediately to remediate HIGH-severity vulnerabilities in production automation.


-- security/pr-reviewer

@louisgvlouisgv added the security-approved Security review approved label Mar 24, 2026
@louisgv
louisgv merged commit e045cf6 into mainMar 24, 2026
6 checks passed
@louisgv
louisgv deleted the fix/security-2961-2962 branch March 24, 2026 18:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-approvedSecurity review approved

Projects

None yet

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962) - #2964

Merged
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962
Mar 24, 2026
Merged

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962)#2964
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: [HIGH] Two security vulnerabilities: (1) Base64 validation bypass in qa.sh safe_substitute allows sed delimiter injection; (2) SPAWN_ISSUE integer validation may not prevent leading zeros/overflow enabling path traversal.

Changes

Fix#1: safe_substitute sed delimiter injection (#2961)

All 4 scripts (qa.sh, refactor.sh, security.sh, discovery.sh):

  • Switch sed delimiter from | to \x01 (SOH control character) which cannot appear in normal text input
  • Remove now-unnecessary | escaping from the value escaping pipeline
  • Add explicit rejection of values containing \x01 as defense-in-depth
  • Keep existing \ and & escaping (required sed metacharacter escaping)

Fix#2: SPAWN_ISSUE integer validation (#2962)

qa.sh:

  • Fix regex from ^[0-9]+$ to ^[1-9][0-9]*$ to reject leading zeros and zero
  • Add 32-bit signed integer range check (max 2,147,483,647 matching GitHub's limit)

refactor.sh, security.sh:

  • Already had ^[1-9][0-9]*$ regex, now also have range validation added

Testing

  • bash -n passes on all 4 modified scripts
  • Functional testing confirms:
    • Pipe characters in values work correctly (previously the delimiter)
    • Backslashes and ampersands are properly escaped
    • Path values substitute correctly
    • \x01 in values is rejected

Fixes#2961
Fixes#2962

-- refactor/security-auditor

… validation
safe_substitute: Switch sed delimiter from | to \x01 (SOH control char) across
qa.sh, refactor.sh, security.sh, and discovery.sh. This eliminates delimiter
injection regardless of value content, since \x01 cannot appear in normal input.
Values containing \x01 are explicitly rejected as defense-in-depth.
SPAWN_ISSUE: Fix qa.sh validation from ^[0-9]+$ to ^[1-9][0-9]*$ to reject
leading zeros and zero itself. Add 32-bit signed integer range check
(max 2147483647) to all three scripts (qa.sh, refactor.sh, security.sh)
to prevent integer overflow in downstream consumers.
Fixes#2961Fixes#2962
Agent: security-auditor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@louisgvlouisgv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: ba294fa

Summary

This PR addresses two HIGH-severity security vulnerabilities in the autonomous agent orchestration scripts. The fixes are well-designed and implement defense-in-depth strategies.

Findings

NONE - All security issues properly addressed:

  1. sed Delimiter Injection (Issue #2961) - FIXED

    • Root Cause: Using pipe character (|) as sed delimiter allowed malicious values containing | to inject arbitrary sed commands
    • Fix: Switch to \x01 (SOH control character) as delimiter, which cannot appear in normal text
    • Defense-in-depth: Added explicit rejection of values containing \x01
    • Impact: All 4 scripts (discovery.sh, qa.sh, refactor.sh, security.sh) consistently patched
  2. Integer Overflow in SPAWN_ISSUE (Issue #2962) - FIXED

    • Root Cause: qa.sh allowed leading zeros (e.g., "042") and didn't enforce GitHub's 32-bit signed int max (2,147,483,647)
    • Fix:
      • Regex updated from ^[0-9]+$ to ^[1-9][0-9]*$ (rejects leading zeros and zero itself)
      • Added length check (max 10 digits) and numeric range check (≤ 2147483647)
    • Consistency: refactor.sh and security.sh already had correct regex, now also have range validation
    • Impact: Prevents path traversal attacks via issue number manipulation

Code Quality

  • No regressions: Removed unnecessary pipe character escaping (no longer needed with \x01 delimiter)
  • Consistent implementation: All 4 scripts use identical safe_substitute() logic
  • Clear documentation: Comments explain the security rationale
  • macOS compatible: grep -qP requires GNU grep (available via brew on macOS, standard on Linux VMs)

Tests

✅ bash -n: PASS on all 4 modified scripts
❌ bun test: N/A (no TypeScript changes)
✅ curl|bash: OK (scripts source from GitHub URLs, no relative paths)
✅ macOS compat: OK (printf instead of echo -e, no bash 4+ features)

Threat Model Assessment

Before: An attacker controlling SPAWN_ISSUE or values passed to safe_substitute could:

  • Execute arbitrary sed commands via delimiter injection
  • Traverse filesystem paths via leading-zero issue numbers

After: Both attack vectors eliminated:

  • \x01 delimiter cannot appear in text input
  • SPAWN_ISSUE strictly validated as 1-2147483647

Residual Risk: None identified

Recommendations

✅ This PR should be merged immediately to remediate HIGH-severity vulnerabilities in production automation.


-- security/pr-reviewer

@louisgvlouisgv added the security-approved Security review approved label Mar 24, 2026
@louisgv
louisgv merged commit e045cf6 into mainMar 24, 2026
6 checks passed
@louisgv
louisgv deleted the fix/security-2961-2962 branch March 24, 2026 18:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-approvedSecurity review approved

Projects

None yet

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962) - #2964

Merged
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962
Mar 24, 2026
Merged

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962)#2964
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: [HIGH] Two security vulnerabilities: (1) Base64 validation bypass in qa.sh safe_substitute allows sed delimiter injection; (2) SPAWN_ISSUE integer validation may not prevent leading zeros/overflow enabling path traversal.

Changes

Fix#1: safe_substitute sed delimiter injection (#2961)

All 4 scripts (qa.sh, refactor.sh, security.sh, discovery.sh):

  • Switch sed delimiter from | to \x01 (SOH control character) which cannot appear in normal text input
  • Remove now-unnecessary | escaping from the value escaping pipeline
  • Add explicit rejection of values containing \x01 as defense-in-depth
  • Keep existing \ and & escaping (required sed metacharacter escaping)

Fix#2: SPAWN_ISSUE integer validation (#2962)

qa.sh:

  • Fix regex from ^[0-9]+$ to ^[1-9][0-9]*$ to reject leading zeros and zero
  • Add 32-bit signed integer range check (max 2,147,483,647 matching GitHub's limit)

refactor.sh, security.sh:

  • Already had ^[1-9][0-9]*$ regex, now also have range validation added

Testing

  • bash -n passes on all 4 modified scripts
  • Functional testing confirms:
    • Pipe characters in values work correctly (previously the delimiter)
    • Backslashes and ampersands are properly escaped
    • Path values substitute correctly
    • \x01 in values is rejected

Fixes#2961
Fixes#2962

-- refactor/security-auditor

… validation
safe_substitute: Switch sed delimiter from | to \x01 (SOH control char) across
qa.sh, refactor.sh, security.sh, and discovery.sh. This eliminates delimiter
injection regardless of value content, since \x01 cannot appear in normal input.
Values containing \x01 are explicitly rejected as defense-in-depth.
SPAWN_ISSUE: Fix qa.sh validation from ^[0-9]+$ to ^[1-9][0-9]*$ to reject
leading zeros and zero itself. Add 32-bit signed integer range check
(max 2147483647) to all three scripts (qa.sh, refactor.sh, security.sh)
to prevent integer overflow in downstream consumers.
Fixes#2961Fixes#2962
Agent: security-auditor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@louisgvlouisgv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: ba294fa

Summary

This PR addresses two HIGH-severity security vulnerabilities in the autonomous agent orchestration scripts. The fixes are well-designed and implement defense-in-depth strategies.

Findings

NONE - All security issues properly addressed:

  1. sed Delimiter Injection (Issue #2961) - FIXED

    • Root Cause: Using pipe character (|) as sed delimiter allowed malicious values containing | to inject arbitrary sed commands
    • Fix: Switch to \x01 (SOH control character) as delimiter, which cannot appear in normal text
    • Defense-in-depth: Added explicit rejection of values containing \x01
    • Impact: All 4 scripts (discovery.sh, qa.sh, refactor.sh, security.sh) consistently patched
  2. Integer Overflow in SPAWN_ISSUE (Issue #2962) - FIXED

    • Root Cause: qa.sh allowed leading zeros (e.g., "042") and didn't enforce GitHub's 32-bit signed int max (2,147,483,647)
    • Fix:
      • Regex updated from ^[0-9]+$ to ^[1-9][0-9]*$ (rejects leading zeros and zero itself)
      • Added length check (max 10 digits) and numeric range check (≤ 2147483647)
    • Consistency: refactor.sh and security.sh already had correct regex, now also have range validation
    • Impact: Prevents path traversal attacks via issue number manipulation

Code Quality

  • No regressions: Removed unnecessary pipe character escaping (no longer needed with \x01 delimiter)
  • Consistent implementation: All 4 scripts use identical safe_substitute() logic
  • Clear documentation: Comments explain the security rationale
  • macOS compatible: grep -qP requires GNU grep (available via brew on macOS, standard on Linux VMs)

Tests

✅ bash -n: PASS on all 4 modified scripts
❌ bun test: N/A (no TypeScript changes)
✅ curl|bash: OK (scripts source from GitHub URLs, no relative paths)
✅ macOS compat: OK (printf instead of echo -e, no bash 4+ features)

Threat Model Assessment

Before: An attacker controlling SPAWN_ISSUE or values passed to safe_substitute could:

  • Execute arbitrary sed commands via delimiter injection
  • Traverse filesystem paths via leading-zero issue numbers

After: Both attack vectors eliminated:

  • \x01 delimiter cannot appear in text input
  • SPAWN_ISSUE strictly validated as 1-2147483647

Residual Risk: None identified

Recommendations

✅ This PR should be merged immediately to remediate HIGH-severity vulnerabilities in production automation.


-- security/pr-reviewer

@louisgvlouisgv added the security-approved Security review approved label Mar 24, 2026
@louisgv
louisgv merged commit e045cf6 into mainMar 24, 2026
6 checks passed
@louisgv
louisgv deleted the fix/security-2961-2962 branch March 24, 2026 18:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-approvedSecurity review approved

Projects

None yet

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962) - #2964

Merged
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962
Mar 24, 2026
Merged

fix(security): prevent sed injection and integer overflow (fixes #2961, #2962)#2964
louisgv merged 1 commit into
mainfrom
fix/security-2961-2962

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: [HIGH] Two security vulnerabilities: (1) Base64 validation bypass in qa.sh safe_substitute allows sed delimiter injection; (2) SPAWN_ISSUE integer validation may not prevent leading zeros/overflow enabling path traversal.

Changes

Fix#1: safe_substitute sed delimiter injection (#2961)

All 4 scripts (qa.sh, refactor.sh, security.sh, discovery.sh):

  • Switch sed delimiter from | to \x01 (SOH control character) which cannot appear in normal text input
  • Remove now-unnecessary | escaping from the value escaping pipeline
  • Add explicit rejection of values containing \x01 as defense-in-depth
  • Keep existing \ and & escaping (required sed metacharacter escaping)

Fix#2: SPAWN_ISSUE integer validation (#2962)

qa.sh:

  • Fix regex from ^[0-9]+$ to ^[1-9][0-9]*$ to reject leading zeros and zero
  • Add 32-bit signed integer range check (max 2,147,483,647 matching GitHub's limit)

refactor.sh, security.sh:

  • Already had ^[1-9][0-9]*$ regex, now also have range validation added

Testing

  • bash -n passes on all 4 modified scripts
  • Functional testing confirms:
    • Pipe characters in values work correctly (previously the delimiter)
    • Backslashes and ampersands are properly escaped
    • Path values substitute correctly
    • \x01 in values is rejected

Fixes#2961
Fixes#2962

-- refactor/security-auditor

… validation
safe_substitute: Switch sed delimiter from | to \x01 (SOH control char) across
qa.sh, refactor.sh, security.sh, and discovery.sh. This eliminates delimiter
injection regardless of value content, since \x01 cannot appear in normal input.
Values containing \x01 are explicitly rejected as defense-in-depth.
SPAWN_ISSUE: Fix qa.sh validation from ^[0-9]+$ to ^[1-9][0-9]*$ to reject
leading zeros and zero itself. Add 32-bit signed integer range check
(max 2147483647) to all three scripts (qa.sh, refactor.sh, security.sh)
to prevent integer overflow in downstream consumers.
Fixes#2961Fixes#2962
Agent: security-auditor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@louisgvlouisgv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: ba294fa

Summary

This PR addresses two HIGH-severity security vulnerabilities in the autonomous agent orchestration scripts. The fixes are well-designed and implement defense-in-depth strategies.

Findings

NONE - All security issues properly addressed:

  1. sed Delimiter Injection (Issue #2961) - FIXED

    • Root Cause: Using pipe character (|) as sed delimiter allowed malicious values containing | to inject arbitrary sed commands
    • Fix: Switch to \x01 (SOH control character) as delimiter, which cannot appear in normal text
    • Defense-in-depth: Added explicit rejection of values containing \x01
    • Impact: All 4 scripts (discovery.sh, qa.sh, refactor.sh, security.sh) consistently patched
  2. Integer Overflow in SPAWN_ISSUE (Issue #2962) - FIXED

    • Root Cause: qa.sh allowed leading zeros (e.g., "042") and didn't enforce GitHub's 32-bit signed int max (2,147,483,647)
    • Fix:
      • Regex updated from ^[0-9]+$ to ^[1-9][0-9]*$ (rejects leading zeros and zero itself)
      • Added length check (max 10 digits) and numeric range check (≤ 2147483647)
    • Consistency: refactor.sh and security.sh already had correct regex, now also have range validation
    • Impact: Prevents path traversal attacks via issue number manipulation

Code Quality

  • No regressions: Removed unnecessary pipe character escaping (no longer needed with \x01 delimiter)
  • Consistent implementation: All 4 scripts use identical safe_substitute() logic
  • Clear documentation: Comments explain the security rationale
  • macOS compatible: grep -qP requires GNU grep (available via brew on macOS, standard on Linux VMs)

Tests

✅ bash -n: PASS on all 4 modified scripts
❌ bun test: N/A (no TypeScript changes)
✅ curl|bash: OK (scripts source from GitHub URLs, no relative paths)
✅ macOS compat: OK (printf instead of echo -e, no bash 4+ features)

Threat Model Assessment

Before: An attacker controlling SPAWN_ISSUE or values passed to safe_substitute could:

  • Execute arbitrary sed commands via delimiter injection
  • Traverse filesystem paths via leading-zero issue numbers

After: Both attack vectors eliminated:

  • \x01 delimiter cannot appear in text input
  • SPAWN_ISSUE strictly validated as 1-2147483647

Residual Risk: None identified

Recommendations

✅ This PR should be merged immediately to remediate HIGH-severity vulnerabilities in production automation.


-- security/pr-reviewer

@louisgvlouisgv added the security-approved Security review approved label Mar 24, 2026
@louisgv
louisgv merged commit e045cf6 into mainMar 24, 2026
6 checks passed
@louisgv
louisgv deleted the fix/security-2961-2962 branch March 24, 2026 18:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-approvedSecurity review approved

Projects

None yet

2 participants

@la14-1@louisgv