Skip to content

Comments

PET: Remove duplicated code#1077

Open
bkchr wants to merge 3 commits intomainfrom
bkchr-remove-duplicates-pet
Open

PET: Remove duplicated code#1077
bkchr wants to merge 3 commits intomainfrom
bkchr-remove-duplicates-pet

Conversation

@bkchr
Copy link
Contributor

@bkchr bkchr commented Feb 19, 2026

  • Does not require a CHANGELOG entry

@bkchr bkchr mentioned this pull request Feb 19, 2026
5 tasks
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Found a few robustness issues in the GitHub Actions workflow around endpoint lookup and template substitution that can lead to invalid YAML or silently starting Subway with no endpoints.

Comment on lines 410 to 417
CHAIN_NAME=$(echo "$chain" | jq -r '.chain')
PORT=$(echo "$chain" | jq -r '.port')

# Get endpoints from runtimes-matrix.json
ENDPOINTS=$(jq -r --arg chain "$CHAIN_NAME" '.[] | select(.name == $chain) | .uris | map("\"" + . + "\"") | join(", ")' .github/workflows/runtimes-matrix.json)

echo "Starting Subway for $CONFIG on port $PORT..."
sed "s/{{PORT}}/$PORT/g" .github/subway-configs/$CONFIG > /tmp/subway-$PORT.yml
echo "Starting Subway for $CHAIN_NAME on port $PORT..."
sed -e "s/{{PORT}}/$PORT/g" -e "s|{{ENDPOINTS}}|$ENDPOINTS|g" .github/subway-configs/template.yml > /tmp/subway-$PORT.yml

Choose a reason for hiding this comment

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

The loop uses ... | while read chain; do (without -r) and then parses JSON from $chain. read will treat backslashes as escapes, which can corrupt JSON strings. Use while IFS= read -r chain; do to ensure the JSON line is read verbatim.

echo "Starting Subway for $CONFIG on port $PORT..."
sed "s/{{PORT}}/$PORT/g" .github/subway-configs/$CONFIG > /tmp/subway-$PORT.yml
echo "Starting Subway for $CHAIN_NAME on port $PORT..."
sed -e "s/{{PORT}}/$PORT/g" -e "s|{{ENDPOINTS}}|$ENDPOINTS|g" .github/subway-configs/template.yml > /tmp/subway-$PORT.yml

Choose a reason for hiding this comment

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

The sed replacement injects $ENDPOINTS directly into the replacement string. If any URI ever contains & (sed replacement expands & to the matched text) or the chosen delimiter (|), the generated YAML will be corrupted. Either escape the replacement (ENDPOINTS_ESCAPED=$(printf '%s' "$ENDPOINTS" | sed 's/[&|\\]/\\&/g')) before sed, or avoid sed entirely by generating the YAML with a templating-safe approach (e.g., have jq emit the full JSON array and substitute that).

@github-actions
Copy link

Review required! Latest push from author must always be reviewed


echo "Starting Subway for $CONFIG on port $PORT..."
sed "s/{{PORT}}/$PORT/g" .github/subway-configs/$CONFIG > /tmp/subway-$PORT.yml
ENDPOINTS=$(jq -c --arg chain "$CHAIN_NAME" '.[] | select(.name == $chain) | .uris' .github/workflows/runtimes-matrix.json)
Copy link
Contributor

@rockbmb rockbmb Feb 20, 2026

Choose a reason for hiding this comment

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

Huh. I must have prompted Claude incorrectly, if I wasn't able to find this file.

Copy link
Member

Choose a reason for hiding this comment

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

The file is here.

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