Skip to content

fix(security): fix command injection and authorization bypass vulnerabilities - #11

Merged
Lin-Jiong-HDU merged 2 commits into
mainfrom
feat/issue-10-fix-security-fix-command-injection-and-a
May 4, 2026
Merged

fix(security): fix command injection and authorization bypass vulnerabilities#11
Lin-Jiong-HDU merged 2 commits into
mainfrom
feat/issue-10-fix-security-fix-command-injection-and-a

Conversation

@Lin-Jiong-HDU

Copy link
Copy Markdown
Owner

Summary

Fixes two HIGH severity security vulnerabilities identified in a security audit:

  1. Command injection in runner-image/entrypoint.sh: The heredoc delimiter was unquoted (<<SCRIPT), causing bash to expand variables at heredoc creation time. An attacker-controlled ISSUE_TITLE containing $(...) or backticks would execute arbitrary commands inside Docker containers. Fixed by quoting the delimiter (<<'SCRIPT') and exporting branch so it's inherited at runtime.

  2. Authorization bypass in .github/workflows/issue-webhook.yml: The workflow only checked if the issue title ended with [claude bot] but did not verify the author's permissions. Any GitHub user could create an issue in a public repo to trigger the full automation pipeline. Fixed by adding author_association check (OWNER/COLLABORATOR/MEMBER) and a server-side author allowlist (github.allowed_authors config) as defense in depth.

Changes

  • runner-image/entrypoint.sh: Quote heredoc delimiter, export branch
  • internal/docker/runner.go: Add SanitizeEnvValue and SanitizeShellValue to strip control chars and shell command substitution patterns from user-controlled env values
  • .github/workflows/issue-webhook.yml: Add author_association permission check
  • domain/config.go: Add AllowedAuthors to GitHubConfig
  • domain/errors.go: Add ErrUnauthorized
  • usecase/task_usecase.go: Author allowlist validation in HandleWebhook
  • api/controller/webhook_controller.go: Return 403 for unauthorized authors
  • bootstrap/bootstrap.go: Wire AllowedAuthors from config

Test plan

  • All pipeline gates pass (go vet, go build, go test, golangci-lint run)
  • New tests for author allowlist validation (allow, reject, empty allowlist)
  • New tests for env value sanitization (control chars, command substitution)
  • New controller test for unauthorized author HTTP response (403)
  • Existing tests updated for new constructor signature

Closes #10

🤖 Generated with Claude Code

…bilities

Two HIGH severity vulnerabilities fixed:

1. Command injection in entrypoint.sh: quote heredoc delimiter to prevent
   premature variable expansion of attacker-controlled ISSUE_TITLE.
   Export `branch` so it is inherited at runtime.

2. Authorization bypass: add author_association check (OWNER/COLLABORATOR/
   MEMBER) in GitHub Actions workflow, and server-side author allowlist
   validation via github.allowed_authors config (defense in depth).

Also adds env value sanitization in runner.go to strip control characters
and shell command substitution patterns from user-controlled fields.

Closes #10

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Addresses two high-severity security findings by hardening the GitHub issue → webhook → Docker runner pipeline, preventing untrusted issue authors from triggering automation and reducing the risk of shell injection when propagating issue content into container execution.

Changes:

  • Hardened runner script generation to prevent heredoc-time expansion and improved quoting around branch/title usage.
  • Added server-side authorization via an optional GitHub author allowlist, wired from config through bootstrap to the usecase/controller.
  • Added sanitization helpers for user-controlled values passed as container env vars, with accompanying tests.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
runner-image/entrypoint.sh Quotes heredoc delimiter and adjusts runtime env usage when generating the launcher script.
internal/docker/runner.go Sanitizes selected task fields before injecting them into container environment variables.
internal/docker/runner_test.go Adds tests covering sanitization behavior.
.github/workflows/issue-webhook.yml Restricts workflow execution to issues opened by trusted author associations.
domain/config.go Adds AllowedAuthors to GitHub config to support server-side allowlist.
domain/errors.go Introduces ErrUnauthorized for authorization failures.
usecase/task_usecase.go Enforces optional author allowlist in HandleWebhook.
usecase/task_usecase_test.go Adds allowlist-related unit tests and updates constructor calls.
api/controller/webhook_controller.go Maps ErrUnauthorized to HTTP 403.
api/controller/webhook_controller_test.go Adds controller test for unauthorized author → 403 behavior.
bootstrap/bootstrap.go Wires github.allowed_authors from config into the usecase constructor.

Comment on lines +50 to +57
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got := docker.SanitizeShellValue(tt.input)
if got != tt.expected {
t.Errorf("SanitizeShellValue(%q) = %q, want %q", tt.input, got, tt.expected)
}
})
Comment thread runner-image/entrypoint.sh Outdated
cd /workspace/repo
git config --global --add safe.directory /workspace/repo
claude -p "\$(cat /tmp/prompt.txt)" --dangerously-skip-permissions
claude -p "$(cat /tmp/prompt.txt)" --dangerously-skip-permissions
Comment on lines +26 to +33
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got := docker.SanitizeEnvValue(tt.input)
if got != tt.expected {
t.Errorf("SanitizeEnvValue(%q) = %q, want %q", tt.input, got, tt.expected)
}
})
…d_authors

The root-path launcher script hardcoded --dangerously-skip-permissions,
ignoring the SKIP_PERMISSIONS setting. Add conditional logic consistent
with the non-root path. Also add allowed_authors to config example and
README.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Lin-Jiong-HDU
Lin-Jiong-HDU merged commit 891d605 into main May 4, 2026
1 check passed
@Lin-Jiong-HDU
Lin-Jiong-HDU deleted the feat/issue-10-fix-security-fix-command-injection-and-a branch May 4, 2026 03:06
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.

fix(security): fix command injection and authorization bypass vulnerabilities [claude bot]

2 participants