Skip to content

Potential fix for environment variable built from user-controlled sources - #38942

Merged
derrickaw merged 2 commits into
masterfrom
alert-autofix-1
Aug 4, 2026
Merged

Potential fix for environment variable built from user-controlled sources#38942
derrickaw merged 2 commits into
masterfrom
alert-autofix-1

Conversation

@derrickaw

@derrickaw derrickaw commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Potential fix for https://github.com/apache/beam/security/code-scanning/1

Use strict parsing + sanitization before writing to $GITHUB_ENV:

  • Read only the first sdk_version= line from gradle.properties.
  • Extract the value safely.
  • Reject values containing CR/LF (prevents env-file line injection).
  • Optionally validate expected version format to keep behavior aligned with intended semantics.
  • Write using printf to avoid shell echo quirks.

Change only .github/workflows/beam_Publish_Beam_SDK_Snapshots.yml in the Find Beam Version step (lines around 90–92). No import/dependency changes are needed.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

run - https://github.com/apache/beam/actions/runs/27416123769
rerun - https://github.com/apache/beam/actions/runs/30821283730

…rces

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@github-actions github-actions Bot added the build label Jun 12, 2026
@derrickaw derrickaw changed the title Potential fix for code scanning alert no. 1: Environment variable built from user-controlled sources Potential fix for environment variable built from user-controlled sources Jun 12, 2026
@derrickaw

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@derrickaw
derrickaw marked this pull request as ready for review June 16, 2026 14:18
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@github-actions

Copy link
Copy Markdown
Contributor

Assigning reviewers:

R: @Abacn for label build.

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

@github-actions

Copy link
Copy Markdown
Contributor

Reminder, please take a look at this pr: @Abacn

@github-actions

Copy link
Copy Markdown
Contributor

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @liferoad for label build.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Reminder, please take a look at this pr: @liferoad

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @Abacn for label build.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

@github-actions

Copy link
Copy Markdown
Contributor

Reminder, please take a look at this pr: @Abacn

@github-actions

Copy link
Copy Markdown
Contributor

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @liferoad for label build.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

@github-actions

Copy link
Copy Markdown
Contributor

Reminder, please take a look at this pr: @liferoad

@bvolpato bvolpato left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for tightening this up. I think this still needs one change before it fixes the alert: grep -q $'[\r\n]' is invalid under GNU grep (Unmatched [; exit 2). Since the if treats that as no match, a CR in sdk_version proceeds to printf and can create another environment-file assignment.

A shell case check (*$'\r'*|*$'\n'*) or valid byte check would reject it. Could you update that guard?

@derrickaw
derrickaw requested a review from bvolpato August 3, 2026 17:43

@bvolpato bvolpato left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@derrickaw
derrickaw merged commit 44d4089 into master Aug 4, 2026
24 checks passed
@derrickaw
derrickaw deleted the alert-autofix-1 branch August 4, 2026 03:23
rwiggles pushed a commit to rwiggles/beam-rwiggles that referenced this pull request Aug 5, 2026
…rces (apache#38942)

* Potential fix for Environment variable built from user-controlled sources

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

* update return/new line guard

---------

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
rwiggles pushed a commit to rwiggles/beam-rwiggles that referenced this pull request Aug 6, 2026
…rces (apache#38942)

* Potential fix for Environment variable built from user-controlled sources

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

* update return/new line guard

---------

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
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.

2 participants