Skip to content

Sanitize branch-derived Docker tag in Makefile targets#80

Closed
DavidJBianco wants to merge 1 commit into
mainfrom
codex/propose-fix-for-command-injection-vulnerability
Closed

Sanitize branch-derived Docker tag in Makefile targets#80
DavidJBianco wants to merge 1 commit into
mainfrom
codex/propose-fix-for-command-injection-vulnerability

Conversation

@DavidJBianco

Copy link
Copy Markdown
Collaborator

Motivation

  • Prevent command injection by avoiding unquoted expansion of the current git branch in Makefile shell recipes used for Docker image tags.

Description

  • Add a DOCKER_TAG variable that derives the branch name via git branch --show-current and normalizes it with tr -c '[:alnum:]._-' '-' to a Docker-tag-safe character set.
  • Replace direct uses of $(shell git branch --show-current) in the container-local and container-run targets with $(DOCKER_TAG).

Testing

  • Ran dry-run checks with make -n container-local and make -n container-run and confirmed both commands emit the sanitized $(DOCKER_TAG) image tag.
  • Both dry-run invocations completed successfully.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 30eb48b686

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Makefile
open htmlcov/index.html || echo "Open htmlcov/index.html in your browser to view the coverage report."

.PHONY: container-local
DOCKER_TAG := $(shell git branch --show-current | tr -c '[:alnum:]._-' '-')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Trim branch newline before sanitizing Docker tag

git branch --show-current emits a trailing newline, and at this line tr -c '[:alnum:]._-' '-' converts that newline into -, so every computed tag becomes <branch>- (for example, work-), not the intended sanitized branch name. This changes the image tag unexpectedly for all container-local/container-run invocations and can break scripts or expectations that rely on branch-aligned tags; trim the newline first (or strip trailing dashes after transliteration).

Useful? React with 👍 / 👎.

@DavidJBianco

Copy link
Copy Markdown
Collaborator Author

Merged into dev via #84 (batch-a-quick-fixes, commit bd1ab9d).

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant