Skip to content

Update playground frontend Dockerfile#4103

Merged
jmg-duarte merged 1 commit intocowprotocol:mainfrom
MarcusWentz:corepack
Jan 30, 2026
Merged

Update playground frontend Dockerfile#4103
jmg-duarte merged 1 commit intocowprotocol:mainfrom
MarcusWentz:corepack

Conversation

@MarcusWentz
Copy link
Copy Markdown
Contributor

Description

Fixes Docker pnpm version error.

Changes

Enable and prepare corepack version before running pnpm install.

Fixes

#4101

@MarcusWentz MarcusWentz requested a review from a team as a code owner January 29, 2026 03:10
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the Dockerfiles to transition from "yarn" to "pnpm", addressing a version error and improving dependency management by enabling Corepack. However, a potential command injection vulnerability was identified in playground/Dockerfile.cowswap due to unquoted build arguments, which could allow arbitrary command execution during the build process. It's recommended to quote these variables to mitigate this risk. Additionally, the apt cache is now cleaned for optimized Docker image size, and shell syntax in Dockerfile.cowswap has been improved with semicolons for robustness.

Comment thread playground/Dockerfile.cowswap Outdated
Copy link
Copy Markdown
Contributor

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

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

LGTM but since you're improving the script I think we can take the chance to group some RUN statements

Comment thread playground/Dockerfile.cowswap Outdated
Comment thread playground/Dockerfile.cowswap Outdated
Comment thread playground/Dockerfile.explorer Outdated
Comment thread playground/Dockerfile.explorer Outdated
@MarcusWentz
Copy link
Copy Markdown
Contributor Author

@jmg-duarte

I grouped some of the RUN commands in the Docker scripts. Does this look good to merge now? Thank you.

Copy link
Copy Markdown
Contributor

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution!

@jmg-duarte jmg-duarte changed the title Refactor Dockerfile to use pnpm and add dependencies Update playground frontend Dockerfile Jan 30, 2026
@jmg-duarte jmg-duarte enabled auto-merge January 30, 2026 14:51
auto-merge was automatically disabled January 30, 2026 14:55

Head branch was pushed to by a user without write access

@jmg-duarte
Copy link
Copy Markdown
Contributor

Please refrain from updating this PR further. It was already in queue and the rebase was unnecessary.

@jmg-duarte jmg-duarte enabled auto-merge January 30, 2026 15:18
@MarcusWentz
Copy link
Copy Markdown
Contributor Author

MarcusWentz commented Jan 30, 2026

Sorry I did a rebase by accident. In the future I will not rebase in this GitHub repo since that cancelled the auto merge.

@jmg-duarte jmg-duarte added this pull request to the merge queue Jan 30, 2026
Merged via the queue into cowprotocol:main with commit 8dd0fdd Jan 30, 2026
19 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 30, 2026
@MarcusWentz MarcusWentz deleted the corepack branch January 30, 2026 15:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants