Skip to content

fix: use unset-safe expansions in entrypoint validation checks#996

Closed
ygd58 wants to merge 1 commit intobase:mainfrom
ygd58:fix/unset-safe-entrypoint-expansions
Closed

fix: use unset-safe expansions in entrypoint validation checks#996
ygd58 wants to merge 1 commit intobase:mainfrom
ygd58:fix/unset-safe-entrypoint-expansions

Conversation

@ygd58
Copy link
Copy Markdown

@ygd58 ygd58 commented Apr 3, 2026

Problem

The entrypoint scripts use set -u which causes bash to exit when an unset variable is expanded. However, the validation logic uses -z "$VAR" which expands the variable before checking if it's empty.

This means if OP_NODE_NETWORK is unset and OP_NODE_ROLLUP_CONFIG is set (for op-node-entrypoint), the script crashes on unbound OP_NODE_NETWORK expansion instead of evaluating the condition correctly.

Fix

Use unset-safe expansions in checks (${VAR:-}) while keeping set -u for safety.

Changes

  • op-node-entrypoint: Use ${OP_NODE_NETWORK:-} and ${OP_NODE_ROLLUP_CONFIG:-}
  • nethermind/nethermind-entrypoint: Use ${OP_NODE_NETWORK:-} and ${OP_NODE_L2_ENGINE_AUTH_RAW:-}

Closes #989

@cb-heimdall
Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

@Kubudak90
Copy link
Copy Markdown

Closing as duplicate of #989 which was opened earlier (March 13) and includes a more complete fix (also covers geth/geth-entrypoint). Thanks for the contribution!

@Kubudak90
Copy link
Copy Markdown

This appears to be a duplicate of #989, which was opened on March 13 and addresses the same issue (#988) with the same fix. Additionally, #989 includes an extra fix for geth/geth-entrypoint that this PR does not cover. Recommend closing this in favor of #989.

@ygd58
Copy link
Copy Markdown
Author

ygd58 commented Apr 5, 2026

Thanks for the review! You're right, this is a duplicate of #989 which has a more complete fix. Closing in favor of that PR.

@ygd58 ygd58 closed this Apr 5, 2026
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.

3 participants