Skip to content

chore: consider switching from py to .sh +...#4499

Open
nickboldt wants to merge 5 commits intomainfrom
replace-py-with-bash-and-js
Open

chore: consider switching from py to .sh +...#4499
nickboldt wants to merge 5 commits intomainfrom
replace-py-with-bash-and-js

Conversation

@nickboldt
Copy link
Copy Markdown
Member

@nickboldt nickboldt commented Mar 27, 2026 •
edited
Loading

What does this PR do?

chore: consider switching from py to .sh + .cjs
generated-by: cursor slop factory

Signed-off-by: Nick Boldt nboldt@redhat.com

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

N/A (or see commit message above for issue number)

How to test this PR?

N/A

PR Checklist

As the author of this Pull Request I made sure that:

  • Code produced is complete
  • Code builds without errors
  • Tests are covering the bugfix
  • Relevant user documentation updated
  • Relevant contributing documentation updated

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

Image was built and published successfully. It is available at:

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

The container image build workflow finished with status: failure.

@nickboldt nickboldt force-pushed the replace-py-with-bash-and-js branch from 1d945d4 to e1e3bf8 Compare March 30, 2026 20:06
@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge bot commented Mar 30, 2026 •
edited
Loading

CI Feedback 🧐

(Feedback updated until commit e1e3bf8)

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: Build with Node.js 22

Failed stage: Check Image and Relevant Changes [❌]

Failed test name: ""

Failure summary:

The action failed because a jq command attempted to parse invalid/non-JSON input and errored with
jq: parse error: Invalid numeric literal at line 1, column 7 (log line 382), causing the step to
exit with code 5 (log line 383).
The environment shows IMAGE_REGISTRY is empty (log line 380), which
likely led to a malformed/empty response from a preceding command that was then piped into jq for
parsing.

Relevant error logs: 
1:  ##[group]Runner Image Provisioner
2:  Hosted Compute Agent
...

367:  �[36;1melif [ -n "$IMAGE_EXISTS_COMMIT" ]; then�[0m
368:  �[36;1m  echo "Image $IMAGE_NAME_COMMIT already exists."�[0m
369:  �[36;1m  echo "exists=true" >> $GITHUB_OUTPUT�[0m
370:  �[36;1melse�[0m
371:  �[36;1m  echo "Image does not exist (checked both $IMAGE_NAME_PR and $IMAGE_NAME_COMMIT)."�[0m
372:  �[36;1m  echo "exists=false" >> $GITHUB_OUTPUT�[0m
373:  �[36;1mfi�[0m
374:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
375:  env:
376:  TURBO_SCM_BASE: 205a6d5bd9f5bd6af00a6c9bf928f2b796385d84
377:  TURBO_SCM_HEAD: 1f99749dbd41372ef4a201728711166713ecdfe3
378:  SHORT_SHA: e1e3bf8d
379:  PR_NUMBER: 4499
380:  IMAGE_REGISTRY: 
381:  ##[endgroup]
382:  jq: parse error: Invalid numeric literal at line 1, column 7
383:  ##[error]Process completed with exit code 5.
384:  Post job cleanup.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

The container image build workflow finished with status: failure.

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 30, 2026
View reviewed changes
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 30, 2026
View reviewed changes
scripts/install-dynamic-plugins/src/install.ts
Comment on lines +41 to +43
execFileSync('sh', ['-c', 'command -v "$1"', 'sh', name], {
stdio: 'ignore'
});

Check warning

Code scanning / CodeQL

Shell command built from environment values Medium

This shell command depends on an uncontrolled
absolute path
Loading
.

Copilot Autofix

AI 1 day ago

In general, to fix this type of issue, avoid spawning a shell (sh -c ...) when checking for or invoking commands whose names or paths may come from the environment or other untrusted sources. Instead, call the desired executable directly with execFileSync/execFile (or similar) and pass arguments as an array, so the runtime bypasses shell parsing entirely.

Here, needCmd only wants to verify that a command or executable exists and is runnable. Rather than invoking sh -c 'command -v "$1"' sh name, we can simply try to run the command directly, e.g. execFileSync(name, ['--version']) or, more generically, run it with no arguments and ignore failures that come from its exit code while still detecting “command not found”. The minimal change that preserves semantics is to call execFileSync(name, ['--version']) (or just execFileSync(name, {stdio:'ignore'})), catch any error, and treat it as “required command not found”. Because the file only needs to know “does this run at all?”, not the command’s output, ignoring its stdio is acceptable.

Concretely, in scripts/install-dynamic-plugins/src/install.ts, update the needCmd function (around lines 39–46) to remove the sh -c 'command -v ...' pattern and instead call execFileSync directly with name as the command. No new imports or helper methods are needed; execFileSync is already imported from node:child_process. All other uses of execFileSync in the shown snippet already pass arguments directly and do not involve a shell, so they can remain unchanged.

Suggested changeset 1
scripts/install-dynamic-plugins/src/install.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/scripts/install-dynamic-plugins/src/install.ts b/scripts/install-dynamic-plugins/src/install.ts
--- a/scripts/install-dynamic-plugins/src/install.ts
+++ b/scripts/install-dynamic-plugins/src/install.ts
@@ -38,9 +38,8 @@
 
 function needCmd(name: string): void {
   try {
-    execFileSync('sh', ['-c', 'command -v "$1"', 'sh', name], {
-      stdio: 'ignore'
-    });
+    // Try to execute the command directly to verify that it exists and is runnable.
+    execFileSync(name, ['--version'], { stdio: 'ignore' });
   } catch {
     die(`required command not found: ${name}`);
   }
EOF
@@ -38,9 +38,8 @@

function needCmd(name: string): void {
try {
execFileSync('sh', ['-c', 'command -v "$1"', 'sh', name], {
stdio: 'ignore'
});
// Try to execute the command directly to verify that it exists and is runnable.
execFileSync(name, ['--version'], { stdio: 'ignore' });
} catch {
die(`required command not found: ${name}`);
}
Copilot is powered by AI and may make mistakes. Always verify output.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

The container image build workflow finished with status: failure.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 31, 2026

The container image build workflow finished with status: failure.

nickboldt and others added 5 commits March 31, 2026 12:52
@nickboldt
chore: consider switching from py to .sh + .cjs
afdea9b 
generated-byt : cursor

Signed-off-by: Nick Boldt <nboldt@redhat.com>
@nickboldt
include jq in runtime env
0cc0932 
Signed-off-by: Nick Boldt <nboldt@redhat.com>
@rhdh-bot @nickboldt
switch from bash and node to all TS all the time
ab4f511 
Signed-off-by: rhdh-bot service account <rhdh-bot@redhat.com>
@nickboldt @github-advanced-security
Potential fix for code scanning alert no. 110: Incomplete string esca…
19c42bc 
…ping or encoding

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@nickboldt
add yarn lock for install-dynamic-plugins project/script
82678c1 
Signed-off-by: Nick Boldt <nboldt@redhat.com>
@nickboldt nickboldt force-pushed the replace-py-with-bash-and-js branch from c97d0a4 to 82678c1 Compare March 31, 2026 15:52
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 31, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
14 Security Hotspots
D Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 31, 2026

The container image build workflow finished with status: failure.

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Mar 31, 2026

@nickboldt: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-ocp-helm 82678c1 link true /test e2e-ocp-helm

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nickboldt @github-advanced-security @rhdh-bot