RHAIENG-6201: Backport hermetic codeserver build to rhoai-3.3#2523
RHAIENG-6201: Backport hermetic codeserver build to rhoai-3.3#2523ysok wants to merge 2 commits into
Conversation
|
@ysok — This PR is from a fork. Recommended: Push your branch to the main repo for full CI: Then open a new PR from that branch. No push access? A maintainer will cherry-pick and test your changes. See CONTRIBUTING.md for details. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR migrates CodeServer to hermetic, offline multi-architecture builds, adds dependency lockfile and prefetch tooling, updates GitHub Actions and Tekton pipelines, configures Podman and runtime networking, and expands validation, artifact reporting, and build documentation. ChangesHermetic CodeServer build
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| # Setup VSCode marketplace extensions and Node.js binaries from prefetched files. | ||
| # VSCODE_OFFLINE_CACHE is already exported by codeserver-offline-env.sh. | ||
| # Built-in .vsix (js-debug, etc.) are in repo at utils/ (COPY'd into image); not from cachi2. | ||
| mkdir -p "${VSCODE_OFFLINE_CACHE}" | ||
| VSIX_UTILS="${CODESERVER_SOURCE_CODE}/utils" | ||
|
|
||
| # Copy .vsix extension files from utils/ (git-tracked large files) |
| # build-vscode.sh is patched to build for current arch (vscode-reh-web-linux-${NODE_ARCH}). | ||
| # On ppc64le/s390x we add those arches to BUILD_TARGETS (below) and run the native task, | ||
| # so we only need system node in linux-${NODE_ARCH} (no extra node tarball). | ||
| NODE_BUILD_VERSION=$(grep -E '^target=' "${CODESERVER_SOURCE_PREFETCH}/lib/vscode/remote/.npmrc" | cut -d'"' -f2) | ||
| NODE_ARCH=$(node -p "process.arch") | ||
| VSCODE_BUILD_DIR="${CODESERVER_SOURCE_PREFETCH}/lib/vscode/.build" | ||
| NODE_CACHE_DIR="${VSCODE_BUILD_DIR}/node/v${NODE_BUILD_VERSION}/linux-${NODE_ARCH}" | ||
| mkdir -p "${NODE_CACHE_DIR}" | ||
| cp /usr/bin/node "${NODE_CACHE_DIR}/node" | ||
| chmod +x "${NODE_CACHE_DIR}/node" | ||
| echo "Cached system node for .build/node/v${NODE_BUILD_VERSION}/linux-${NODE_ARCH}/node" | ||
|
|
||
| # --- Pre-populate built-in extensions from prefetched .vsix files --- | ||
| # builtInExtensions.js: isUpToDate() checks .build/builtInExtensions/<name>/package.json |
| fi | ||
|
|
||
| echo "Extracting ${ext_name} from $(basename "${vsix_file}")..." | ||
| local tmp_dir="/tmp/vsix-extract-$$" |
| # Print gomod prefetch-input paths from a Tekton file (one per line). | ||
| extract_gomod_paths_from_tekton() { | ||
| local tekton_file="$1" | ||
| yq eval ' |
| cp "$PYLOCK_FILE" "${PROJECT_DIR}/uv.lock.d/pylock.${FLAVOR}.toml" | ||
| PYLOCK_FILE="${PROJECT_DIR}/uv.lock.d/pylock.${FLAVOR}.toml" |
| podman run --rm -i $TTY_FLAG \ | ||
| -v "$PWD:$PWD:z" \ |
| rm -f "$REG_LOG" | ||
| rm -rf "$CDN_CERT_DIR" | ||
| error_exit "Failed to obtain entitlement certs. Check cert-dir, activation key, or org." | ||
| fi | ||
| rm -f "$REG_LOG" | ||
|
|
||
| echo " cert: $CDN_CERT" | ||
| echo " key: $CDN_KEY" | ||
| echo " CA: $CDN_CA" | ||
|
|
||
| # Remap host paths to container mount paths — the cert dir is mounted | ||
| # at /certs inside the hermeto container (see -v flag below). | ||
| C_CERT="/certs${CDN_CERT#"$CDN_CERT_DIR"}" | ||
| C_KEY="/certs${CDN_KEY#"$CDN_CERT_DIR"}" | ||
| C_CA="/certs${CDN_CA#"$CDN_CERT_DIR"}" | ||
|
|
||
| HERMETO_JSON=$(jq -n \ | ||
| --arg cert "$C_CERT" \ | ||
| --arg key "$C_KEY" \ | ||
| --arg ca "$C_CA" \ | ||
| '{"type":"rpm","options":{"ssl":{"client_cert":$cert,"client_key":$key,"ca_bundle":$ca}}}') | ||
| fi | ||
|
|
||
| # ========================================================================= | ||
| # Download RPMs and generate repo metadata. | ||
| # | ||
| # hermeto fetch-deps wipes its --output directory on every run, so we use | ||
| # a staging dir to avoid destroying pip/npm/generic artifacts that other | ||
| # prefetch steps already placed in cachi2/output/deps/. | ||
| # ========================================================================= | ||
| HERMETO_STAGING=$(mktemp -d) | ||
| trap 'rm -rf "$HERMETO_STAGING" ${CDN_CERT_DIR:+"$CDN_CERT_DIR"}' EXIT | ||
|
|
||
| echo "--- Downloading RPMs via hermeto ---" | ||
| podman run --rm \ | ||
| -v "$(pwd)/$PREFETCH_DIR:/source:z" \ | ||
| -v "$HERMETO_STAGING:/output:z" \ | ||
| ${CDN_CERT_DIR:+-v "$CDN_CERT_DIR:/certs:ro,z"} \ |
| rm -rf "$HERMETO_OUTPUT/deps/rpm" | ||
| mv "$HERMETO_STAGING/deps/rpm" "$HERMETO_OUTPUT/deps/rpm" | ||
| cp -f "$HERMETO_STAGING/bom.json" "$HERMETO_OUTPUT/bom.json" | ||
| cp -f "$HERMETO_STAGING/.build-config.json" "$HERMETO_OUTPUT/.build-config.json" | ||
|
|
||
| echo "Finished! RPMs are in $HERMETO_OUTPUT/deps/rpm" |
|
|
||
| if [[ "$USE_SUBSCRIPTION_REPOS" == true ]]; then | ||
| # RHOAI/AIPCC runtime images use RHEL 9.6 EUS repos (baseos/appstream/CRB). | ||
| basearch=$(rpm --eval '%{_arch}') |
| # ========================================================================= | ||
| echo "=== [5/5] Go modules ===" | ||
| if [[ -n "$tekton_file" ]] && command -v yq &>/dev/null; then | ||
| gomod_paths=$(yq eval ' |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## rhoai-3.3 #2523 +/- ##
===========================================
Coverage ? 0
===========================================
Files ? 0
Lines ? 0
Branches ? 0
===========================================
Hits ? 0
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Trivy found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
| if index_host == wheel_host: | ||
| return True | ||
| # PyPI simple index; wheels are served from files.pythonhosted.org. | ||
| if "pypi.org" in index_host and wheel_host == "files.pythonhosted.org": |
| if "pypi.org" in index_host and wheel_host == "files.pythonhosted.org": | ||
| return True | ||
| # RH simple index; wheels are served from pulp-content host. | ||
| if "packages.redhat.com" in index_host and "packages.redhat.com" in wheel_host: |
| if "pypi.org" in index_host and wheel_host == "files.pythonhosted.org": | ||
| return True | ||
| # RH simple index; wheels are served from pulp-content host. | ||
| if "packages.redhat.com" in index_host and "packages.redhat.com" in wheel_host: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.tekton/odh-workbench-codeserver-datascience-cpu-py312-pull-request.yaml (1)
12-203: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMove this PipelineRun update to
konflux-central..tekton/in this repo is synced from that source, so a direct edit here will be overwritten on the next sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.tekton/odh-workbench-codeserver-datascience-cpu-py312-pull-request.yaml around lines 12 - 203, Move the PipelineRun definition identified by `odh-workbench-codeserver-datascience-cpu-py312-on-pull-request-75964` to the corresponding `konflux-central` source configuration, rather than updating this synced `.tekton` file directly. Ensure the source retains the existing labels, parameters, platforms, and prefetch-input entries.Sources: Coding guidelines, Learnings
🟠 Major comments (30)
.tekton/odh-workbench-codeserver-datascience-cpu-py312-v3-3-push.yaml-3-3 (1)
3-3: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftApply these PipelineRun changes in
konflux-central, not here.This generated
.tekton/file will be overwritten unless the hermetic parameters and prefetch inputs are first updated in the synchronization source.As per coding guidelines, “Do not modify
.tekton/inred-hat-data-services/notebooksdirectly; PipelineRuns are synced fromkonflux-central.”Also applies to: 24-50, 61-199, 215-216
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.tekton/odh-workbench-codeserver-datascience-cpu-py312-v3-3-push.yaml at line 3, Do not modify the generated PipelineRun configuration in this .tekton file; revert these changes and apply the required hermetic parameters and prefetch inputs in the konflux-central synchronization source instead.Source: Coding guidelines
scripts/index_url_resolver.py-123-125 (1)
123-125: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not silently resolve RHOAI 3.3 builds against a newer release index.
If the configured 3.3 index probe fails,
_resolve_from_releasetries 3.5 first and returns it as a successful resolution. This can generate a nominally 3.3 image with packages built for a different RHOAI release; fail explicitly or restrict fallbacks to approved compatible releases.Also applies to: 317-336
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/index_url_resolver.py` around lines 123 - 125, Update RHOAI_INDEX_FALLBACK_RELEASES and the fallback logic in _resolve_from_release so a failed RHOAI 3.3 index probe cannot resolve against the newer 3.4 or 3.5 indexes. Restrict fallback candidates to explicitly approved compatible releases, or fail resolution when none are compatible, while preserving valid fallback behavior for other releases.scripts/lockfile-generators/README.md-42-50 (1)
42-50: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUpdate the workflow documentation from rhoai-2.25/3.5 to RHOAI 3.3.
These sections prescribe the old public-PyPI overlay flow and even a 3.5 base/index, while this backport uses
rh-indexand the RHOAI 3.3 AIPCC configuration. Following them can regenerate incompatible locks or build against the wrong release.Also applies to: 110-128, 149-158, 374-376, 395-407, 797-818, 857-874, 1070-1072
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/README.md` around lines 42 - 50, Update the workflow documentation throughout README sections around the Python lockfile instructions, including the referenced ranges, from rhoai-2.25/3.5 to RHOAI 3.3. Replace old public-PyPI overlay, base-image, and index guidance with the rh-index and RHOAI 3.3 AIPCC configuration used by this backport, while preserving the documented commands and outputs where applicable.scripts/lockfile-generators/create-requirements-lockfile.sh-118-125 (1)
118-125: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve the resolved RH index in
requirements.<flavor>.txt.RH mode resolves the configured RHOAI index at Line 120, but Line 124 replaces the Hermeto index with public PyPI, and Line 213 prioritizes that value. RH-only wheels in the generated lock will consequently fail during Cachi2/Hermeto prefetch.
Proposed fix
- HERMETO_INDEX_URL="https://pypi.org/simple" + HERMETO_INDEX_URL="$REQUIREMENTS_INDEX_URL"Also applies to: 213-220
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/create-requirements-lockfile.sh` around lines 118 - 125, Preserve the resolved REQUIREMENTS_INDEX_URL for RH mode when generating requirements.<flavor>.txt: update the HERMETO_INDEX_URL assignment and the later index-selection logic around the lockfile generation path so they prioritize REQUIREMENTS_INDEX_URL instead of replacing it with public PyPI. Keep PyPI as the fallback only when no RH index is resolved, while retaining the existing extra-index behavior.scripts/lockfile-generators/prefetch-all.sh-84-105 (1)
84-105: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply distinct Tekton matching rules for ODH and RHDS.
The
odh || rhdscondition is always true here, and both variants only matchDockerfile.konflux.*. ODH components usingDockerfile.*therefore lose their NPM and Go prefetch discovery.Proposed fix
- if [[ "$variant" == "odh" ]] || [[ "$variant" == "rhds" ]]; then + if [[ "$variant" == "rhds" ]]; then if [[ "$dockerfile_path" == "$comp_dir/Dockerfile.konflux."* ]]; then echo "$f" found=1 fi + elif [[ "$variant" == "odh" ]]; then + if [[ "$dockerfile_path" == "$comp_dir/Dockerfile."* && + "$dockerfile_path" != "$comp_dir/Dockerfile.konflux."* ]]; then + echo "$f" + found=1 + fi fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/prefetch-all.sh` around lines 84 - 105, Update find_tekton_yaml so the variant-specific matching rules are distinct: for "odh", match Dockerfile paths under comp_dir using Dockerfile.* while excluding Dockerfile.konflux.*; for "rhds", retain matching Dockerfile.konflux.*. Ensure each matching rule echoes the file and sets found, while unsupported variants do not match.scripts/pylocks_generator.py-521-542 (1)
521-542: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOnly require Konflux flavors in
rh-indexmode.Flavor detection currently rejects the directory before
effective_modeis calculated. Consequently,public-indexcannot process a valid Python project unless it also happens to containDockerfile.konflux.*.Proposed approach
- flavors = detect_flavors(tdir) - if not flavors: - log.warning(f"No Dockerfile.konflux.* files found in {tdir} (cpu/cuda/rocm). Skipping.") - return tdir, False, log - - log.print(f"📦 Python version: {python_version}") - log.print("🧩 Detected flavors:") - for f in sorted(flavors): - log.print(f" • {f.upper()}") - log.print("") - if index_mode == IndexMode.auto: effective_mode = IndexMode.rh_index if (tdir / "uv.lock.d").is_dir() else IndexMode.public_index else: effective_mode = index_mode + + flavors = detect_flavors(tdir) + if effective_mode == IndexMode.rh_index and not flavors: + log.warning(f"No Dockerfile.konflux.* files found in {tdir} (cpu/cuda/rocm). Skipping.") + return tdir, False, log🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/pylocks_generator.py` around lines 521 - 542, Update the directory-processing flow around effective_mode so it is determined before flavor validation. Require detected Konflux flavors only when effective_mode is IndexMode.rh_index; allow public-index processing to continue without Dockerfile.konflux.* files, while preserving the existing warning and early return for rh-index directories with no flavors.scripts/lockfile-generators/Dockerfile.rpm-lockfile-11-12 (1)
11-12: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winProvide a valid default base image.
ARG BASE_IMAGEhas no default, so${BASE_IMAGE}is syntactically invalid during Dockerfile validation even when callers normally pass it. Give it the branch’s intended AIPCC base default and allow callers to override it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/Dockerfile.rpm-lockfile` around lines 11 - 12, Update the BASE_IMAGE argument declaration in the Dockerfile so it uses the branch’s intended AIPCC base image as its default while preserving caller overrides through build arguments. Keep the existing FROM ${BASE_IMAGE} reference unchanged.Source: Pipeline failures
codeserver/ubi9-python-3.12/README.md-52-73 (1)
52-73: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftThe documentation still describes the RHOAI 2.25 build contract. Adapt both guides to the PR’s RHOAI 3.3 AIPCC base, RH 3.3 Python index, and separate subscription/public RPM-lock flows.
codeserver/ubi9-python-3.12/README.md#L52-L73: replace the 2.25 build configuration and lockfile commands with the actual 3.3 variants and inputs.codeserver/README.md#L32-L40: update the branch notes and overview to match those 3.3 settings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/README.md` around lines 52 - 73, Update codeserver/ubi9-python-3.12/README.md lines 52-73 to document the RHOAI 3.3 AIPCC base image, RH 3.3 Python index, and separate subscription/public RPM-lock flows, including the correct build targets, lockfile inputs, and regeneration commands. Update codeserver/README.md lines 32-40 so its branch notes and overview match the same RHOAI 3.3 settings; both locations require direct documentation changes.scripts/lockfile-generators/Dockerfile.rpm-lockfile-15-24 (1)
15-24: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftPass the activation key as a build secret, not an
ARG.The static-analysis pipeline rejects
ARG ACTIVATION_KEY, and build arguments can expose credentials through build metadata or logs. Mount the activation key as a secret for the registration step and update the invoking script accordingly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/Dockerfile.rpm-lockfile` around lines 15 - 24, Replace the ACTIVATION_KEY build argument with a BuildKit secret mount and read the key only within the registration RUN step, while retaining ORG and release pinning behavior. Update the corresponding lockfile-generation script invocation to pass the activation key via the build secret mechanism instead of a build argument.Source: Pipeline failures
codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/lib/extensions.ts-374-378 (1)
374-378: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse the low-memory merge helper here too.
es.merge()bypassesmergeExtensionStreams(), so native and non-native packaging still run concurrently on ppc64/s390x and can reintroduce the OOM risk this helper is meant to avoid.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/lib/extensions.ts` around lines 374 - 378, Update packageAllLocalExtensionsStream to use the existing mergeExtensionStreams helper instead of es.merge, passing the native and non-native extension streams so packaging runs with the intended low-memory sequencing.scripts/lockfile-generators/helpers/hermeto-fetch-gomod.sh-73-83 (1)
73-83: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not silently accept a failed Go-module merge.
Both
cpoperations discard all errors, defeatingset -e. A permission or disk failure can produce an empty/incomplete cache while Line 88 reports success. Copy directory contents without glob suppression and fail when no dependency output is found.Proposed fix
if [[ -d "$HERMETO_STAGING/deps/gomod" ]]; then - cp -a "$HERMETO_STAGING/deps/gomod"/* "$HERMETO_OUTPUT/deps/gomod/" 2>/dev/null || true + cp -a "$HERMETO_STAGING/deps/gomod/." "$HERMETO_OUTPUT/deps/gomod/" elif [[ -d "$HERMETO_STAGING/deps" ]]; then - # Some hermeto versions may use a different subdir; merge whatever is under deps/ + found=false for sub in "$HERMETO_STAGING/deps"/*/; do - [[ -d "$sub" ]] && cp -a "$sub"* "$HERMETO_OUTPUT/deps/gomod/" 2>/dev/null || true + [[ -d "$sub" ]] || continue + found=true + cp -a "$sub/." "$HERMETO_OUTPUT/deps/gomod/" done + "$found" || error_exit "Hermeto produced no Go module dependencies" +else + error_exit "Hermeto produced no dependency output" fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/helpers/hermeto-fetch-gomod.sh` around lines 73 - 83, Update the Go-module merge logic around the staging-copy branches so cp failures propagate instead of being masked by “|| true” and redirected errors. Copy the staged dependency contents without suppressing glob or filesystem errors, and validate that at least one dependency output was found/copied; otherwise exit nonzero rather than allowing the successful completion path to continue. Preserve support for both deps/gomod and alternate subdirectories under deps/.codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/lib/extensions.ts-84-95 (1)
84-95: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon't skip type checking on ppc64/s390x. Esbuild only bundles; this branch should still run the
typeCheckExtensionStreampass so low-memory builds don’t ship TypeScript errors that other architectures catch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/lib/extensions.ts` around lines 84 - 95, Update the hasEsbuild branch to run typeCheckExtensionStream for LOW_MEMORY_ARCHES as well, rather than assigning input directly from fromLocalEsbuild. Preserve the existing getBuildRootsForExtension, mergeExtensionStreams, and esbuild bundling flow so ppc64/s390x receive the same type-check pass as other architectures..github/workflows/build-notebooks-TEMPLATE.yaml-399-399 (1)
399-399: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPropagate the resolved
PRODUCT, not the raw optional input.When
inputs.productis omitted, Line 399 exports an emptyPRODUCT, overriding the job default and preventingMakefile’sPRODUCT ?= rhoaifrom applying. This can select the wrong build-args configuration.
.github/workflows/build-notebooks-TEMPLATE.yaml#L399-L399: remove the step override or use${{ env.PRODUCT }}..github/workflows/build-notebooks-TEMPLATE.yaml#L362-L362: test$PRODUCTrather than${{ inputs.product }}.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-notebooks-TEMPLATE.yaml at line 399, Use the resolved PRODUCT value throughout the workflow: at .github/workflows/build-notebooks-TEMPLATE.yaml:399-399, remove the step-level override or set it from env.PRODUCT instead of raw inputs.product; at .github/workflows/build-notebooks-TEMPLATE.yaml:362-362, test the shell variable $PRODUCT rather than inputs.product so the default remains effective when the input is omitted..github/workflows/build-notebooks-TEMPLATE.yaml-303-303 (1)
303-303: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPass expression values through environment variables instead of generated executable source.
Quotes or shell metacharacters in these values are parsed as Python/shell code after GitHub performs template expansion.
.github/workflows/build-notebooks-TEMPLATE.yaml#L303-L303: expose the target throughenvand reados.environ["TARGET"]..github/workflows/build-notebooks-TEMPLATE.yaml#L340-L340: use the existing quoted$MAKE_TARGET..github/workflows/build-notebooks-TEMPLATE.yaml#L362-L362: compare the quoted$PRODUCT..github/workflows/build-notebooks-TEMPLATE.yaml#L394-L394: pass a quoted environment variable tomake..github/workflows/build-notebooks-TEMPLATE.yaml#L458-L458: pass the target throughenv..github/workflows/build-notebooks-TEMPLATE.yaml#L491-L491: pass the target throughenv..github/workflows/build-notebooks-TEMPLATE.yaml#L592-L592: passOUTPUT_IMAGEthroughenv.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-notebooks-TEMPLATE.yaml at line 303, Stop interpolating expression values into generated Python or shell source in .github/workflows/build-notebooks-TEMPLATE.yaml: at lines 303-303 and 458-458 and 491-491, expose the target through env and read it from the environment; at line 340-340 reuse the existing quoted $MAKE_TARGET; at line 362-362 compare the quoted $PRODUCT; at line 394-394 pass the quoted environment variable to make; and at line 592-592 pass OUTPUT_IMAGE through env. Preserve the existing conditional and build behavior while ensuring all values are treated as data.Source: Linters/SAST tools
codeserver/ubi9-python-3.12/Dockerfile.konflux.cpu-18-18 (1)
18-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGive the global
BASE_IMAGEargument a valid default.Both
FROM ${BASE_IMAGE}instructions currently fail theInvalidDefaultArgInFrompipeline rule. The explicit build argument can still override this default.Proposed fix
-ARG BASE_IMAGE +ARG BASE_IMAGE=registry.access.redhat.com/ubi9/python-312:latest🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/Dockerfile.konflux.cpu` at line 18, Set a valid default value for the global BASE_IMAGE argument in Dockerfile.konflux.cpu so both FROM ${BASE_IMAGE} instructions resolve during validation, while preserving the ability for explicit build arguments to override it.Source: Pipeline failures
codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/npm/preinstall.ts-143-160 (1)
143-160: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard the optional local header metadata before dereferencing it.
getHeaderInfo()can returnundefined, but Line 150 unconditionally accesseslocal.target. A Linux install with an incomplete local.npmrctherefore crashes after the earlier guarded install step.Proposed fix
- if (process.platform === 'linux') { + if (process.platform === 'linux' && local !== undefined) { const homedir = os.homedir(); const cachePath = process.env.XDG_CACHE_HOME || path.join(homedir, '.cache'); const nodeGypCache = path.join(cachePath, 'node-gyp'); - const localHeaderPath = path.join(nodeGypCache, local!.target, 'include', 'node'); + const localHeaderPath = path.join(nodeGypCache, local.target, 'include', 'node');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/npm/preinstall.ts` around lines 143 - 160, Guard the Linux header-patching block against missing local header metadata before accessing local.target. Update the condition around localHeaderPath, using the existing local value from getHeaderInfo(), so patching is skipped when local is undefined while preserving the current behavior when valid metadata exists..github/actions/install-podman-action/action.yml-79-92 (1)
79-92: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExport the socket variables for this step.
$GITHUB_ENVonly updates subsequent steps, so these sanity checks and network reload target rootless Podman instead of the configured rootful socket.Proposed fix
podman_sock=/var/run/podman/podman.sock + export PODMAN_SOCK="${podman_sock}" + export CONTAINER_HOST="unix://${podman_sock}" echo "PODMAN_SOCK=${podman_sock}" >> "$GITHUB_ENV" echo "CONTAINER_HOST=unix://${podman_sock}" >> "$GITHUB_ENV"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/actions/install-podman-action/action.yml around lines 79 - 92, Export podman_sock, PODMAN_SOCK, and CONTAINER_HOST in the current shell before the podman ps and podman network reload commands, while retaining the GITHUB_ENV writes for subsequent steps. Ensure both sanity checks use the configured rootful socket rather than the default rootless Podman endpoint.codeserver/ubi9-python-3.12/prefetch-input/patches/copy-gha-native-bindings.sh-64-68 (1)
64-68: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRequire the node-pty binding. The optional path can let this step succeed without a native addon in
release-standalone/lib/vscode/node_modules/node-pty, which would break terminal support. Make it required, or add an explicit post-copy check that a.nodefile exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/patches/copy-gha-native-bindings.sh` around lines 64 - 68, Update the copy_native_artifacts invocation for node-pty so the native binding is required rather than optional, or add an explicit post-copy validation that a .node file exists under the destination node-pty directory before succeeding.codeserver/ubi9-python-3.12/requirements.cpu.txt-1229-1236 (1)
1229-1236: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRegenerate the vendor wheel URLs for RHOAI 3.3.
Both
ripgrepanduvare locked fromrhoai/3.5/cpu-ubi9, contradicting this backport’s RHOAI 3.3 target. Use the 3.3 repository and regenerate all architecture-specific hashes.Also applies to: 1461-1468
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/requirements.cpu.txt` around lines 1229 - 1236, Update the architecture-specific ripgrep and uv wheel entries in the requirements lockfiles to use the rhoai/3.3/cpu-ubi9 repository instead of rhoai/3.5/cpu-ubi9, then regenerate every platform-specific SHA256 hash for those wheels. Preserve the existing Python and architecture markers.scripts/lockfile-generators/create-artifact-lockfile.py-56-66 (1)
56-66: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDownload atomically to avoid accepting partial cache files.
A failed
wget -O target_pathcan leave a truncated file. The next invocation sees it as cached and locks it without retrying. Download to a temporary path, delete it on failure, then atomically replace the target after success.Also applies to: 121-128
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/create-artifact-lockfile.py` around lines 56 - 66, Update download_file to download into a temporary sibling path instead of target_path, remove that temporary file when wget fails, and atomically replace target_path only after a successful download. Preserve the existing wget options and error behavior, and ensure cleanup also covers subprocess failures before the replacement occurs.Source: Linters/SAST tools
scripts/lockfile-generators/helpers/download-pip-packages.py-390-395 (1)
390-395: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFail when any required package resolves to zero artifacts.
HTTP errors, unsupported index HTML, or unmatched hashes currently become
[], and Phase 3 silently continues. This can produce an incomplete cache that only fails later during the offline build. Preserve the package identity and exit if any non-skipped requirement has no matching files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/helpers/download-pip-packages.py` around lines 390 - 395, Update the Phase 3 parallel resolve flow around resolve_one and executor.map so any non-skipped requirement resolving to zero artifacts causes an immediate failure. Preserve each requirement’s package identity alongside its result, detect empty file lists (including HTTP, unsupported-index, and hash-mismatch cases), and exit with a clear error instead of silently extending all_files; retain existing handling for skipped requirements.Source: Linters/SAST tools
codeserver/ubi9-python-3.12/run-code-server.sh-124-132 (1)
124-132: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAccount for IPv6-only socket mode.
When
/proc/sys/net/ipv6/bindv6onlyis1, binding[::]:8787does not accept IPv4 connections despite the dual-stack message. Select the IPv4 fallback in that case.Proposed fix
-if [ -f /proc/net/if_inet6 ] && [ "$(cat /proc/sys/net/ipv6/conf/all/disable_ipv6 2>/dev/null)" != "1" ]; then +if [ -f /proc/net/if_inet6 ] \ + && [ "$(cat /proc/sys/net/ipv6/conf/all/disable_ipv6 2>/dev/null)" != "1" ] \ + && [ "$(cat /proc/sys/net/ipv6/bindv6only 2>/dev/null)" != "1" ]; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/run-code-server.sh` around lines 124 - 132, Update the IPv6 detection condition in the BIND_ADDR selection block to require that /proc/sys/net/ipv6/bindv6only is not 1, falling back to 0.0.0.0:8787 when IPv6-only socket mode is enabled while preserving the existing IPv6 and IPv4 messages.scripts/lockfile-generators/create-artifact-lockfile.py-131-143 (1)
131-143: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject checksum mismatches instead of blessing new bytes.
The supplied checksum is described as validation, but a mismatch only warns and the lockfile records the unexpected artifact’s hash. Abort generation and remove the mismatching cache entry.
Proposed fix
if checksum.lower() != expected: - print(f" ⚠ Warning: Checksum mismatch for {filename}", file=sys.stderr) - print(f" Expected: {expected[:16]}...", file=sys.stderr) - print(f" Got: {checksum[:16]}...", file=sys.stderr) + cache_file.unlink(missing_ok=True) + raise RuntimeError( + f"Checksum mismatch for {filename}: " + f"expected {expected}, got {checksum}" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/create-artifact-lockfile.py` around lines 131 - 143, Update the checksum validation in the artifact-generation flow around provided_checksum and normalize_checksum so a mismatch aborts lockfile generation instead of returning the downloaded artifact. Remove the mismatching cache entry before terminating, while preserving successful validation and the existing lockfile return structure for matching checksums or absent supplied checksums.scripts/lockfile-generators/rewrite-npm-urls.sh-126-127 (1)
126-127: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAllow periods in GitHub repository names.
([^.#"]+)stops at the first period, so valid dependencies such asowner/package.js.git#refare not rewritten and remain unavailable during an offline build.Proposed fix
-([^/]+)/([^.#"]+)(?:\.git)?#([^"]+) +([^/]+)/([^/#"]+?)(?:\.git)?#([^"]+)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/rewrite-npm-urls.sh` around lines 126 - 127, Update the Perl URL-matching expression in the git+ssh/git+https rewrite step to allow periods in the GitHub repository capture, while still stopping at the repository/ref separator and closing quote. Preserve the existing optional .git suffix handling and ref sanitization in the rewrite.scripts/lockfile-generators/helpers/patch-rh-wheel-only-packages.py-24-27 (1)
24-27: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRecognize
manylinuxandmusllinuxwheel tags here.wheel_arch_key()returnsunknownformanylinux2014_ppc64le,manylinux_2_17_s390x, andmusllinux_1_2_*wheels, somerge-bekeeps those PyPI BE artifacts instead of replacing them with the RH wheels.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/helpers/patch-rh-wheel-only-packages.py` around lines 24 - 27, Update the _WHEEL_ARCH_RE pattern used by wheel_arch_key() to recognize manylinux and musllinux platform tags while preserving extraction of x86_64, aarch64, ppc64le, and s390x. Ensure variants such as manylinux2014_ppc64le, manylinux_2_17_s390x, and musllinux_1_2_* return the correct architecture instead of unknown.scripts/lockfile-generators/helpers/rpm-lockfile-generate.sh-53-59 (1)
53-59: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReuse the committed checksum for the EPEL key instead of bootstrapping it blindly.
artifacts.lock.yamlalready pinsRPM-GPG-KEY-EPEL-9, but this path still downloads it withcurl -sLand never checks the bytes. Add--failand compare against the committed sha256 before trusting it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/helpers/rpm-lockfile-generate.sh` around lines 53 - 59, Update the EPEL key bootstrap block around EPEL_KEY to download with curl failure handling and validate the downloaded file against the committed SHA-256 checksum for RPM-GPG-KEY-EPEL-9 from artifacts.lock.yaml before accepting it. Reject and remove the file when validation fails, and only print the success message after checksum verification passes.codeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.sh-90-91 (1)
90-91: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse unique temporary files for each
jqrewrite.Because this script runs as root, predictable
/tmp/*.jsonpaths permit symlink-based file overwrite and collide across concurrent executions. Create each output withmktemp, then move it into place.Example fix
- jq 'del(.packages["node_modules/agent-browser"].integrity)' \ - lib/vscode/package-lock.json > /tmp/lock-agent-browser.json \ - && mv /tmp/lock-agent-browser.json lib/vscode/package-lock.json + tmpfile=$(mktemp) + jq 'del(.packages["node_modules/agent-browser"].integrity)' \ + lib/vscode/package-lock.json > "$tmpfile" + mv "$tmpfile" lib/vscode/package-lock.jsonAlso applies to: 113-115, 129-132, 146-147, 161-162, 172-176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.sh` around lines 90 - 91, Update each jq rewrite in the patch script, including the loop processing the package-lock files and the other referenced rewrite blocks, to create its output with mktemp rather than a predictable /tmp filename. Pass the unique temporary path to jq, then move that file into the target only after jq succeeds, preserving the existing cleanup and replacement behavior.Source: Linters/SAST tools
scripts/lockfile-generators/helpers/pylock-to-requirements.py-233-250 (1)
233-250: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDerive the index marker from the actual off-index architectures.
Any off-index wheel currently forces the indexed requirement to
x86_64/aarch64. If an off-index wheel targets either architecture, that architecture receives both a direct URL and a version pin; if it targets another combination, valid index architectures can be omitted.Build the exclusion marker from
wheel_arch_key()results rather than assuming every off-index wheel is big-endian.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/helpers/pylock-to-requirements.py` around lines 233 - 250, Update the indexed-requirement marker logic near le_only_marker(marker) to derive exclusions from wheel_arch_key() for each off-index wheel’s actual architecture. Ensure architectures represented by off-index wheels receive only their direct URL while other valid architectures retain the indexed version pin, including mixed architecture combinations.scripts/lockfile-generators/helpers/pylock-to-requirements.py-85-98 (1)
85-98: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winCompare parsed hostnames on DNS-label boundaries.
Substring checks treat unrelated hosts such as
evilpypi.orgorpackages.redhat.com.attacker.exampleas trusted. This can misclassify an off-index wheel as index-served and alter which URL pip/Hermeto retrieves.Possible fix
- if "pypi.org" in index_host and wheel_host == "files.pythonhosted.org": + if index_host == "pypi.org" and wheel_host == "files.pythonhosted.org": return True - if "packages.redhat.com" in index_host and "packages.redhat.com" in wheel_host: + if ( + (index_host == "packages.redhat.com" or index_host.endswith(".packages.redhat.com")) + and (wheel_host == "packages.redhat.com" or wheel_host.endswith(".packages.redhat.com")) + ): return True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/helpers/pylock-to-requirements.py` around lines 85 - 98, Update index_url_serves_wheel to compare hostnames on DNS-label boundaries rather than using substring checks. Ensure PyPI and Red Hat host matching accepts the intended domain or subdomains but rejects lookalikes such as evilpypi.org and packages.redhat.com.attacker.example, while preserving the existing files.pythonhosted.org wheel-host behavior.Source: Linters/SAST tools
scripts/lockfile-generators/create-rpm-lockfile.sh-147-160 (1)
147-160: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not reuse the generator image across different configurations.
The fixed
localhost/notebook-rpm-lockfile:latesttag ignoresBASE_IMAGE,RHEL_VERSION, and subscription mode. After one invocation, subsequent ODH/RHDS runs can silently generate locks using stale repositories.Use a configuration-derived tag or rebuild unconditionally.
Possible fix
-if podman image exists localhost/notebook-rpm-lockfile:latest 2>/dev/null; then - echo "--- Reusing existing Lockfile Generator Image (rm it to rebuild after changing base/subscription) ---" -else - echo "--- Building Lockfile Generator Image ---" - podman build \ +echo "--- Building Lockfile Generator Image ---" +podman build \ -f "$SCRIPTS_PATH/Dockerfile.rpm-lockfile" \ --platform=linux/x86_64 \ --build-arg RHEL_VERSION="$RHEL_VERSION" \ --build-arg BASE_IMAGE="$BASE_IMAGE" \ --build-arg ACTIVATION_KEY="$ACTIVATION_KEY" \ --build-arg ORG="$ORG" \ ${CACHE_ARGS+"${CACHE_ARGS[@]}"} \ -t notebook-rpm-lockfile "$SCRIPTS_PATH" -fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/create-rpm-lockfile.sh` around lines 147 - 160, The image reuse check in the lockfile generator must distinguish configurations instead of always using localhost/notebook-rpm-lockfile:latest. Update the image tag and corresponding build/reuse references in the generator flow to derive from BASE_IMAGE, RHEL_VERSION, and subscription mode (including ACTIVATION_KEY/ORG), or remove reuse and rebuild unconditionally; ensure each invocation cannot use an image built for another configuration.
🟡 Minor comments (10)
codeserver/ubi9-python-3.12/prefetch-input/patches/tweak-gha.sh-24-48 (1)
24-48: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAssert the replacements land
sed -isucceeds on zero matches, so upstream drift can leave these GHA memory tweaks unapplied without failing the script. Add post-patch checks for the expected values, especially thebuild_from_source="true"→falseedit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/patches/tweak-gha.sh` around lines 24 - 48, Add post-patch validation to tweak-gha.sh for every memory-related replacement, including the build_from_source change in the npmrc loop, NODE_HEAP_MB or max-old-space-size, mangle worker limits, and transpiler worker cap. Check that each target file contains the expected replacement value and fail the script with a clear error if any check does not pass, so upstream drift cannot silently skip these tweaks.scripts/lockfile-generators/README.md-316-316 (1)
316-316: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the invalid pip-section fragment.
The target heading at Line 790 does not contain
RHOAI, so this fragment does not resolve.Proposed fix
-| 4 | pip (RHOAI) | [create-requirements-lockfile.sh](`#4-pip-packages-rhoai--create-requirements-lockfilesh`) | `pylock.<flavor>.toml` + `requirements.<flavor>.txt` | +| 4 | pip (RHOAI) | [create-requirements-lockfile.sh](`#4-pip-packages--create-requirements-lockfilesh`) | `pylock.<flavor>.toml` + `requirements.<flavor>.txt` |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/README.md` at line 316, Update the pip (RHOAI) table link target to match the actual heading for create-requirements-lockfile.sh, removing the unmatched RHOAI fragment so the Markdown anchor resolves correctly.Source: Linters/SAST tools
codeserver/ubi9-python-3.12/README.md-349-365 (1)
349-365: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument the same
cachi2/outputmount used by the build commands.Line 351 claims the Makefile mounts
./cachi2at/cachi2, while Lines 241-242 and the manual command mount./cachi2/outputat/cachi2/output. Use the actual mount in this explanation to prevent incorrect manual builds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/README.md` around lines 349 - 365, Update the “Why the cachi2 volume mount?” section to document mounting ./cachi2/output at /cachi2/output, matching the Makefile and manual build commands. Revise the local-build explanation and any resulting path wording so it consistently describes the /cachi2/output mount.codeserver/ubi9-python-3.12/README.md-83-88 (1)
83-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the repository-relative submodule path.
This command block invokes
./scripts/..., so it must run from the repository root, whereprefetch-input/code-serveris not a valid path.-git submodule update --init --recursive prefetch-input/code-server +git submodule update --init --recursive codeserver/ubi9-python-3.12/prefetch-input/code-server🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/README.md` around lines 83 - 88, Update the README command block so it runs from the repository root, using the repository-relative submodule path expected by ./scripts/lockfile-generators/prefetch-all.sh; adjust the submodule update command and related component reference consistently without changing the build workflow.scripts/lockfile-generators/download-npm.sh-106-110 (1)
106-110: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSanitize Git refs before using them as filenames.
feature/foobecomesowner-repo-feature/foo.tgz, which turns the slash into a nested path and breakswget. Keep the raw ref in the GitHub archive URL, but normalize it before building the.tgzname in both GitHub ref paths here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/download-npm.sh` around lines 106 - 110, Update the GitHub ref handling in the displayed URL-matching branch and the other GitHub ref path to sanitize refs before constructing the .tgz filename, replacing path separators such as / with a filename-safe character. Preserve the raw ref when building the GitHub archive URL so refs like feature/foo continue downloading correctly.scripts/lockfile-generators/create-go-lockfile.sh-78-81 (1)
78-81: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject an explicit directory without
go.sum.The documented
--prefetch-dircontract requires both files, but a missinggo.sumis later skipped and the command can exit successfully without fetching anything.Proposed fix
if [[ -n "$PREFETCH_DIR" ]]; then [[ -d "$PREFETCH_DIR" ]] || error_exit "Prefetch directory not found: $PREFETCH_DIR" [[ -f "$PREFETCH_DIR/go.mod" ]] || error_exit "go.mod not found in $PREFETCH_DIR" + [[ -f "$PREFETCH_DIR/go.sum" ]] || error_exit "go.sum not found in $PREFETCH_DIR" fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/create-go-lockfile.sh` around lines 78 - 81, Update the PREFETCH_DIR validation block to require a regular go.sum file alongside go.mod when an explicit prefetch directory is provided. Use error_exit with a clear missing-file message, while preserving the existing directory and go.mod checks.scripts/lockfile-generators/rewrite-npm-urls.sh-39-41 (1)
39-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve an externally configured
CACHI2_BASE.The unconditional assignment prevents the documented HTTPS mirror mode.
Proposed fix
-CACHI2_BASE="file:///cachi2/output/deps/npm" +CACHI2_BASE="${CACHI2_BASE:-file:///cachi2/output/deps/npm}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/rewrite-npm-urls.sh` around lines 39 - 41, Update the CACHI2_BASE assignment in the rewrite-npm-urls script to use the existing externally configured value when provided, while retaining file:///cachi2/output/deps/npm as the default. Preserve the variable’s shell expansion behavior for the Perl commands and allow HTTPS mirror configuration to remain effective.tests/containers/kubernetes_utils.py-270-275 (1)
270-275: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd a timeout to the
requests.getcall.Currently,
requests.getis called without a timeout. If the pod's port-forward connection accepts the TCP connection but hangs without returning an HTTP response, this call will block indefinitely, preventing theWait.untilmechanism from enforcing its 30-second timeout.⏳ Proposed fix
Wait.until( "Connecting to pod succeeds", 1, 30, - lambda: requests.get(f"http://localhost:{self.port}").status_code == 200, + lambda: requests.get(f"http://localhost:{self.port}", timeout=2).status_code == 200, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/containers/kubernetes_utils.py` around lines 270 - 275, Add an explicit timeout to the requests.get call inside the “Connecting to pod succeeds” Wait.until condition, using a short duration appropriate for each polling attempt so a hung HTTP response cannot bypass Wait.until’s 30-second limit.scripts/lockfile-generators/helpers/rpm-lockfile-generate.sh-20-27 (1)
20-27: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle the documented
--prefetch-input=option.The documented form currently produces a path such as
/workspace/--prefetch-input=..., causingpushdto fail.Proposed fix
- prefetch-input=*|PREFETCH_INPUT_DIR=*) + --prefetch-input=*|prefetch-input=*|PREFETCH_INPUT_DIR=*) PREFETCH_INPUT_DIR="${arg#*=}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/helpers/rpm-lockfile-generate.sh` around lines 20 - 27, Update the argument parsing case in the lockfile generator so the documented --prefetch-input= option is recognized and its value is assigned to PREFETCH_INPUT_DIR without the option prefix. Preserve existing support for prefetch-input=, PREFETCH_INPUT_DIR=, and positional arguments.tests/test_main.py-47-48 (1)
47-48: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLimit the exemption to the RPM lockfile Dockerfile.
Skipping the entire directory prevents this guard from detecting accidental
subscription-manager registerusage in other generator images. Exempt onlyDockerfile.rpm-lockfile.Proposed fix
- if file.is_relative_to(PROJECT_ROOT / "scripts/lockfile-generators"): + if file == PROJECT_ROOT / "scripts/lockfile-generators/Dockerfile.rpm-lockfile": continue # RPM lockfile image optionally uses subscription-manager🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_main.py` around lines 47 - 48, Limit the path exemption in the test guard to the specific Dockerfile.rpm-lockfile under scripts/lockfile-generators, rather than skipping the entire directory; keep other generator files subject to subscription-manager register detection.
🧹 Nitpick comments (1)
tests/containers/base_image_test.py (1)
32-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the generic exception catch to preserve stack traces.
Catching
Exceptionand failing viapytest.faildiscards the original stack trace, making debugging significantly harder when a test fails. Let Pytest handle the exception natively so it can present the full traceback.♻️ Proposed refactor
def _run_test(self, image: str, test_fn: Callable[[docker_utils.NotebookContainer], None]): - try: - with docker_utils.running_container(image) as container: - test_fn(container) - except Exception as e: - pytest.fail(f"Unexpected exception in test: {e}") + with docker_utils.running_container(image) as container: + test_fn(container)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/containers/base_image_test.py` around lines 32 - 36, Remove the broad Exception handler and pytest.fail wrapper around the running_container/test_fn block, allowing exceptions from test_fn and the container context manager to propagate directly to Pytest with their original tracebacks.
| if (srcExists) { | ||
| if (!(await fsExists(BIN_PATH))) { | ||
| await mkdir(BIN_PATH); | ||
| } | ||
| await copyFile(ripgrepBinaryPath, RG_BINARY); | ||
| await chmod(RG_BINARY, 0o755); | ||
| console.log(`Using ripgrep from RIPGREP_BINARY_PATH: ${ripgrepBinaryPath}`); | ||
| process.exit(0); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Prevent silent installation failures by handling promise rejections.
Because this file registers a custom unhandledRejection listener (lines 28-30) that does not exit the process, any error thrown by await copyFile or await chmod will be swallowed. The script will exit with code 0, causing the build to report success even if the ripgrep binary failed to copy, resulting in a broken code-server instance.
Wrap the file operations in a try-catch block to ensure failures explicitly terminate the process with a non-zero exit code.
🛠️ Proposed fix
if (srcExists) {
- if (!(await fsExists(BIN_PATH))) {
- await mkdir(BIN_PATH);
- }
- await copyFile(ripgrepBinaryPath, RG_BINARY);
- await chmod(RG_BINARY, 0o755);
- console.log(`Using ripgrep from RIPGREP_BINARY_PATH: ${ripgrepBinaryPath}`);
- process.exit(0);
+ try {
+ if (!(await fsExists(BIN_PATH))) {
+ await mkdir(BIN_PATH);
+ }
+ await copyFile(ripgrepBinaryPath, RG_BINARY);
+ await chmod(RG_BINARY, 0o755);
+ console.log(`Using ripgrep from RIPGREP_BINARY_PATH: ${ripgrepBinaryPath}`);
+ process.exit(0);
+ } catch (err) {
+ console.error(`Failed to copy ripgrep from RIPGREP_BINARY_PATH: ${err}`);
+ process.exit(1);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (srcExists) { | |
| if (!(await fsExists(BIN_PATH))) { | |
| await mkdir(BIN_PATH); | |
| } | |
| await copyFile(ripgrepBinaryPath, RG_BINARY); | |
| await chmod(RG_BINARY, 0o755); | |
| console.log(`Using ripgrep from RIPGREP_BINARY_PATH: ${ripgrepBinaryPath}`); | |
| process.exit(0); | |
| } | |
| if (srcExists) { | |
| try { | |
| if (!(await fsExists(BIN_PATH))) { | |
| await mkdir(BIN_PATH); | |
| } | |
| await copyFile(ripgrepBinaryPath, RG_BINARY); | |
| await chmod(RG_BINARY, 0o755); | |
| console.log(`Using ripgrep from RIPGREP_BINARY_PATH: ${ripgrepBinaryPath}`); | |
| process.exit(0); | |
| } catch (err) { | |
| console.error(`Failed to copy ripgrep from RIPGREP_BINARY_PATH: ${err}`); | |
| process.exit(1); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/ripgrep/postinstall.js`
around lines 101 - 109, Wrap the asynchronous file operations in the srcExists
branch of the postinstall flow, including mkdir, copyFile, and chmod, in a
try-catch so any failure explicitly exits with a non-zero status. Keep the
success log and process.exit(0) only after all operations complete successfully.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-notebooks-pr.yaml (1)
57-74: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExclude RHEL matrix entries for fork PRs.
Line 66 always includes RHEL images, so
build-odhcan dispatch subscription-dependent jobs for forks despite the workflow stating they are skipped. Selectexcludewhenis_forkis true.Proposed fix
- name: Determine targets to build based on changed files run: | set -x + RHEL_IMAGES=include + if [[ "${IS_FORK}" == "true" ]]; then + RHEL_IMAGES=exclude + fi git fetch --no-tags origin "pull/${PR_NUMBER}/head:${HEAD_REF}" git fetch --no-tags origin "+refs/heads/${BASE_REF}:refs/remotes/origin/${BASE_REF}" python3 ci/cached-builds/gen_gha_matrix_jobs.py \ --from-ref "origin/${BASE_REF}" \ --to-ref "${HEAD_REF}" \ - --rhel-images include \ + --rhel-images "${RHEL_IMAGES}" \ --s390x-images include env: + IS_FORK: ${{ steps.fork-check.outputs.is_fork }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-notebooks-pr.yaml around lines 57 - 74, Update the “Determine targets to build based on changed files” step and its gen_gha_matrix_jobs.py invocation so the RHEL images option resolves to exclude when is_fork is true and include otherwise. Preserve the existing s390x-images behavior and ensure the fork-sensitive value is passed through the step’s environment without relying on unsafe GitHub Actions expression interpolation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-notebooks-pr-rhel.yaml:
- Around line 38-43: Update the head repository lookup in the workflow script to
use optional chaining when accessing
context.payload.pull_request.head.repo.full_name, so deleted fork repositories
yield undefined instead of throwing. Keep the existing base repository lookup
and same-repository comparison behavior unchanged.
---
Outside diff comments:
In @.github/workflows/build-notebooks-pr.yaml:
- Around line 57-74: Update the “Determine targets to build based on changed
files” step and its gen_gha_matrix_jobs.py invocation so the RHEL images option
resolves to exclude when is_fork is true and include otherwise. Preserve the
existing s390x-images behavior and ensure the fork-sensitive value is passed
through the step’s environment without relying on unsafe GitHub Actions
expression interpolation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1fef48bc-fcef-4e7c-ac02-ab678ab35dc6
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
.github/actions/capture-kernel-logs/action.yml.github/workflows/build-notebooks-TEMPLATE.yaml.github/workflows/build-notebooks-pr-aipcc.yaml.github/workflows/build-notebooks-pr-rhel.yaml.github/workflows/build-notebooks-pr.yaml.github/workflows/build-notebooks-push.yaml.github/workflows/test-build-notebooks-template.yamlci/cached-builds/containers.confci/cached-builds/podman.serviceci/cached-builds/podman.socketci/logging_config.pyjupyter/minimal/ubi9-python-3.12/Dockerfile.cpujupyter/minimal/ubi9-python-3.12/Dockerfile.cudajupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cpujupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cudajupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.rocmjupyter/minimal/ubi9-python-3.12/Dockerfile.rocmjupyter/minimal/ubi9-python-3.12/prefetch-input/odh/rpms.in.yamljupyter/minimal/ubi9-python-3.12/prefetch-input/odh/rpms.lock.yamljupyter/minimal/ubi9-python-3.12/prefetch-input/repos/centos.repojupyter/minimal/ubi9-python-3.12/prefetch-input/repos/epel.repojupyter/minimal/ubi9-python-3.12/prefetch-input/repos/openshift-clients.repojupyter/minimal/ubi9-python-3.12/prefetch-input/repos/rhsm-pulp.repojupyter/minimal/ubi9-python-3.12/prefetch-input/repos/ubi.repojupyter/minimal/ubi9-python-3.12/pylock.tomljupyter/minimal/ubi9-python-3.12/pyproject.tomljupyter/utils/install_pdf_deps.shpyproject.tomlscripts/buildinputs/buildinputs_test.goscripts/lockfile-generators/create-requirements-lockfile.shtests/containers/docker_utils.py
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/lockfile-generators/create-requirements-lockfile.sh
- .github/workflows/build-notebooks-TEMPLATE.yaml
|
/build-codeserver |
d89e8c8 to
a6c1f33
Compare
|
Caution There are some errors in your PipelineRun template.
|
a6c1f33 to
dabe797
Compare
|
/build-codeserver |
dabe797 to
6ac8628
Compare
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-notebooks-TEMPLATE.yaml (1)
286-322: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winGitHub Actions template injection:
${{ inputs.target }}is interpolated directly into a Python literal.Line 303 embeds
${{ inputs.target }}inside a double-quoted Python string literal in ashell: pythonblock. Because Actions expression expansion happens at workflow-compile time (before the interpreter ever runs), any"or other Python-syntax character ininputs.targetbreaks out of the string literal and is executed as arbitrary Python in this step — this is exactly what zizmor'stemplate-injectionrule flags.inputs.targetis derived from Makefile target names in the checked-out tree, which a PR can modify, so this is attacker-influenceable.The fix already used elsewhere in this same file (see Line 378
MAKE_TARGET: ${{ inputs.target }}and the safe pattern inbuild-notebooks-pr-aipcc.yaml/build-notebooks-pr-rhel.yamlat "RHAIENG-3914: use env vars to prevent GitHub Actions expression injection") is to pass the value viaenv:and reference it as a variable, never interpolate${{ }}inside the script body.🔒 Proposed fix
run: | import os import sys from typing import Literal extra_podman_build_args = "" if "${{ inputs.platform }}" == "linux/s390x": ... - if "codeserver" in "${{ inputs.target }}": + if "codeserver" in os.environ["TARGET"]: ... + env: + TARGET: ${{ inputs.target }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-notebooks-TEMPLATE.yaml around lines 286 - 322, Update the Python step identified by “Compute CONTAINER_BUILD_CACHE_ARGS” to pass inputs.target through the step’s env mapping and read it from os.environ, rather than interpolating the GitHub expression inside the Python string used by the codeserver check. Preserve the existing codeserver argument behavior while applying the established MAKE_TARGET-style safe pattern.Source: Linters/SAST tools
♻️ Duplicate comments (1)
.github/workflows/build-notebooks-pr-rhel.yaml (1)
41-42: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd optional chaining for deleted-fork PRs in both
authorizejobs. Both workflows contain the identicalcontext.payload.pull_request.head.repo.full_nameline without?.; if the source fork was deleted,head.repoisnulland this throws, failing the job. This was already raised onbuild-notebooks-pr-rhel.yamlin a prior review and remains unfixed there, and the same script was duplicated verbatim intobuild-notebooks-pr-aipcc.yaml.
.github/workflows/build-notebooks-pr-rhel.yaml#L41-L42: changecontext.payload.pull_request.head.repo.full_nametocontext.payload.pull_request.head.repo?.full_name..github/workflows/build-notebooks-pr-aipcc.yaml#L41-L42: apply the identical?.fix to the copy of this script.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-notebooks-pr-rhel.yaml around lines 41 - 42, Update the authorize job scripts in .github/workflows/build-notebooks-pr-rhel.yaml lines 41-42 and .github/workflows/build-notebooks-pr-aipcc.yaml lines 41-42 to use optional chaining when accessing head.repo.full_name, preserving the existing headRepo and baseRepo assignments while allowing deleted-fork pull requests to proceed without throwing.
🧹 Nitpick comments (6)
Makefile (1)
106-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated build-args parser between
build_imageandbuild_hermetic_image.The awk-based
.conf→--build-argparsing block (including thePARSE_FAILEDerror handling) is copied verbatim frombuild_image(lines 85-98). Any future fix/tweak to this parser now has to be applied in two places, and it's easy to let them drift.Consider extracting the shared parsing logic into its own macro (e.g.
parse_build_args_conf) that bothbuild_imageandbuild_hermetic_imagecall.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 106 - 163, Extract the duplicated awk-based conf parsing and PARSE_FAILED handling from build_image and build_hermetic_image into a shared parse_build_args_conf macro. Update both image-build macros to call it and reuse its generated build-args value, preserving the current validation, quoting, and error behavior.scripts/lockfile-generators/helpers/patch-rh-wheel-only-packages.py (1)
216-216: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused unpacked variable
ref_header.Ruff (RUF059) flags this; prefix with underscore to signal intent.
🧹 Proposed fix
- ref_header, ref_wheel_lines, _ = parse_wheels_section(ref_map[name]) + _ref_header, ref_wheel_lines, _ = parse_wheels_section(ref_map[name])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/helpers/patch-rh-wheel-only-packages.py` at line 216, Update the unpacking assignment in the wheel-processing logic around parse_wheels_section to prefix the unused ref_header variable with an underscore, preserving the existing use of ref_wheel_lines and behavior.Source: Linters/SAST tools
scripts/lockfile-generators/README.md (1)
42-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winREADME documents the
rhoai-2.25pip strategy, but this PR backports torhoai-3.3.The "Python lockfiles" and "rhoai-2.25 codeserver pip strategy" sections describe
PUBLIC_INDEX_PROJECTSlistingcodeserverand locking against public PyPI — butcreate-requirements-lockfile.shon this branch hasPUBLIC_INDEX_PROJECTS=()(empty) with an explicit comment that "rhoai-3.3+ uses RH index ... (rh-index mode)". As written, a reader on thisrhoai-3.3branch would expect the public-index flow to apply to codeserver, which it doesn't here. Consider retitling/rewording these sections to reflect the branch this file actually lives in (or explicitly note they describe a different branch's configuration for reference).Also applies to: 110-129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/README.md` around lines 42 - 51, Update the README sections describing the Python lockfiles and codeserver pip strategy to match the rhoai-3.3 branch configuration, where PUBLIC_INDEX_PROJECTS is empty and create-requirements-lockfile.sh uses RH index mode. Retitle or clearly label any rhoai-2.25-specific guidance as reference-only, and ensure the documented commands, projects, and lockfile paths do not imply codeserver uses public PyPI on this branch.codeserver/ubi9-python-3.12/prefetch-input/patches/setup-offline-binaries.sh (1)
107-110: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse
mktemp -dinstead of a PID-based temp dir name.
/tmp/vsix-extract-$$is predictable; a symlink/race attacker could pre-create the path.mktemp -dgets a kernel-guaranteed unique, unpredictable directory.🔒 Proposed fix
- local tmp_dir="/tmp/vsix-extract-$$" - rm -rf "${tmp_dir}" - mkdir -p "${tmp_dir}" + local tmp_dir + tmp_dir=$(mktemp -d /tmp/vsix-extract-XXXXXX)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/patches/setup-offline-binaries.sh` around lines 107 - 110, Replace the PID-based temporary directory creation in the extraction flow with mktemp -d, assigning its generated path to tmp_dir and preserving the existing cleanup and mkdir behavior as appropriate. Ensure the directory is created uniquely and unpredictably before extracting the VSIX.Source: Linters/SAST tools
codeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.sh (2)
18-20: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueUnsupported-arch fallback is effectively unreachable.
ARCHcan only ever resolve to one of the 4 values checked at line 20 (or blow up earlier underset -uifuname -mreturns anything else), so the "Unsupported architecture" message at line 193 never actually fires for a genuinely unsupported host — the script instead dies with an unbound-variable error before reaching it.🛡️ Proposed fix
-ARCH="${UNAME_TO_GOARCH[$(uname -m)]}" +ARCH="${UNAME_TO_GOARCH[$(uname -m)]:-}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.sh` around lines 18 - 20, Update the ARCH lookup near the architecture validation condition to handle unmapped uname -m values safely, preserving an empty or explicit unsupported value instead of triggering set -u. Ensure those values fall through to the existing “Unsupported architecture” handling later in the script, while retaining the current mappings for amd64, arm64, ppc64le, and s390x.
90-176: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valuePredictable
/tmpfilenames for intermediatejqoutput.Lines 90-91, 113-115, 131-132, 146-147, 161-162, and 174-176 all write intermediate JSON to fixed
/tmp/lock-*.json//tmp/pkg-*.jsonpaths beforemv-ing them back. Static analysis flags this as CWE-377 (predictable temp file). Real risk is low in a root-run, single-tenant hermetic build container, butmktempis a one-line, standard-practice fix for all six occurrences.♻️ Example fix (apply the same pattern at each flagged occurrence)
- jq 'del(.packages["node_modules/@vscode/ripgrep"].integrity)' "$lock" > /tmp/lock-ripgrep.json && mv /tmp/lock-ripgrep.json "$lock" + tmpfile="$(mktemp)" + jq 'del(.packages["node_modules/@vscode/ripgrep"].integrity)' "$lock" > "$tmpfile" && mv "$tmpfile" "$lock"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.sh` around lines 90 - 176, Replace every fixed /tmp intermediate filename used by jq in the patching flow with a securely created temporary file via mktemp, covering the lock and package JSON updates in the agent-browser, parcel watcher, and vsce-sign sections. Preserve each existing jq transformation and mv behavior, and clean up the temporary files as appropriate.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-notebooks-pr.yaml:
- Around line 76-84: Update the “Annotate PR for fork guidance” step so failure
to resolve the shallow-checkout 3-dot range does not abort the job. Make the git
diff lookup tolerate errors and fall back to CONTRIBUTING.md when it cannot
produce a file, while preserving the existing first-file selection and warning
annotation behavior.
In @.github/workflows/build-notebooks-TEMPLATE.yaml:
- Around line 480-489: Update the “Run image tests” step identified by
makefile-tests to pass inputs.target through its env block as TARGET, then
invoke make_test.py using the shell-safe $TARGET variable instead of directly
interpolating the workflow expression in run.
- Around line 332-458: Replace all direct `${{ inputs.target }}` interpolations
in the shell bodies for the prefetch error message, the “Build: make” step, and
the “Check if we have tests or not” step with a quoted environment variable.
Reuse the existing MAKE_TARGET environment value where available, and add the
same env mapping to the other steps before referencing it in their commands.
In
@.tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-v3-3-push.yaml:
- Around line 58-68: Revert the changes in the `.tekton` pipeline definition,
including the `rhel-subscription-activation-key` value, and leave this
repository’s generated pipeline file unchanged. Apply the pipeline update in the
referenced `konflux-central` repository instead.
In `@codeserver/Extensions.md`:
- Line 9: In the extension package description, replace the redundant phrase
“inside of” with “inside” while preserving the surrounding explanation and
examples.
In `@codeserver/README.md`:
- Line 11: Update the codeserver README overview and the “RHOAI 2.25 notes”
section to reflect the 3.3 build target and AIPCC CPU base configured in
konflux.cpu.conf. Remove or revise stale RHOAI 2.25 labels and guidance while
preserving the existing directory overview structure.
In
`@codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/gulpfile.reh.ts`:
- Around line 61-63: Update the Node download path to translate Linux `ppc64` to
`ppc64le` before constructing the archive URL, while leaving the existing
`s390x` and other architecture mappings unchanged. Locate the fallback logic
that uses the platform and arch values for the Node tarball name.
In
`@codeserver/ubi9-python-3.12/prefetch-input/patches/setup-offline-binaries.sh`:
- Around line 58-65: Update the ripgrep lookup in setup-offline-binaries.sh so a
missing rg does not terminate the script under set -e. Make the which rg lookup
non-fatal before the existing RIPGREP_BINARY_PATH validation, preserving the
custom error message and exit behavior.
In `@codeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.lock.yaml`:
- Around line 4-20: Update the repoid fields in the non-x86_64 package blocks,
including the aarch64 entries under arches, to use the repository identifier
matching each block’s target architecture instead of x86_64. Preserve the
existing package URLs and all other lockfile metadata.
In `@codeserver/ubi9-python-3.12/pyproject.toml`:
- Around line 33-38: Update the dependency compilation configuration or
invocation associated with the pyproject.toml [tool.uv] settings to pass
--exclude py-spy to uv pip compile. Ensure py-spy remains excluded during
dependency compilation, matching the existing exclude-dependencies
configuration.
In `@codeserver/ubi9-python-3.12/README.md`:
- Around line 1-27: Update the README’s release references from RHOAI 2.25 and
RELEASE=2.25 to the rhoai-3.3/RHOAI 3.3 values used by the scripts and .tekton
pipelines, including the Python lockfiles anchor. In the manual-build
instructions, replace the public UBI9 Python base image with the AIPCC CPU base
image adopted by this backport, keeping the documented commands and links
consistent with 3.3.
In `@codeserver/ubi9-python-3.12/uv.lock.d/pylock.cpu.toml`:
- Around line 404-406: The lockfile pins multiple vulnerable packages, including
cryptography, urllib3, protobuf, onnx, tornado, pyjwt, idna, starlette, and
virtualenv. Update the relevant upstream dependency pins or
../../dependencies/cve-constraints.txt to require the stated patched versions,
then regenerate the lockfile using its documented command so all resolved
entries reflect secure releases.
In `@scripts/lockfile-generators/create-artifact-lockfile.py`:
- Around line 56-66: Update process_artifact to fail hard on checksum mismatch
and never return or record the computed hash when it differs from the expected
checksum; only emit the artifact lock entry after validation succeeds. Update
download_file to remove target_path when wget raises FileNotFoundError or
CalledProcessError, preventing failed or partial downloads from being reused as
cache hits.
In `@scripts/lockfile-generators/create-rpm-lockfile.sh`:
- Around line 151-159: Update the create-rpm-lockfile podman build and
Dockerfile.rpm-lockfile to stop passing ACTIVATION_KEY and ORG as build
arguments. Provide both values through named --secret mounts, remove their ARG
declarations, and have the relevant build command read credentials from
/run/secrets/<name> while preserving the existing build behavior.
In `@scripts/lockfile-generators/download-npm.sh`:
- Around line 98-112: Update the GitHub URL regex in the git+ssh/git+https
parsing loop to allow dots in repository names, while still excluding delimiters
and quote characters. After extracting the repository in this block, remove the
optional trailing .git suffix before constructing the filename and archive URL.
---
Outside diff comments:
In @.github/workflows/build-notebooks-TEMPLATE.yaml:
- Around line 286-322: Update the Python step identified by “Compute
CONTAINER_BUILD_CACHE_ARGS” to pass inputs.target through the step’s env mapping
and read it from os.environ, rather than interpolating the GitHub expression
inside the Python string used by the codeserver check. Preserve the existing
codeserver argument behavior while applying the established MAKE_TARGET-style
safe pattern.
---
Duplicate comments:
In @.github/workflows/build-notebooks-pr-rhel.yaml:
- Around line 41-42: Update the authorize job scripts in
.github/workflows/build-notebooks-pr-rhel.yaml lines 41-42 and
.github/workflows/build-notebooks-pr-aipcc.yaml lines 41-42 to use optional
chaining when accessing head.repo.full_name, preserving the existing headRepo
and baseRepo assignments while allowing deleted-fork pull requests to proceed
without throwing.
---
Nitpick comments:
In `@codeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.sh`:
- Around line 18-20: Update the ARCH lookup near the architecture validation
condition to handle unmapped uname -m values safely, preserving an empty or
explicit unsupported value instead of triggering set -u. Ensure those values
fall through to the existing “Unsupported architecture” handling later in the
script, while retaining the current mappings for amd64, arm64, ppc64le, and
s390x.
- Around line 90-176: Replace every fixed /tmp intermediate filename used by jq
in the patching flow with a securely created temporary file via mktemp, covering
the lock and package JSON updates in the agent-browser, parcel watcher, and
vsce-sign sections. Preserve each existing jq transformation and mv behavior,
and clean up the temporary files as appropriate.
In
`@codeserver/ubi9-python-3.12/prefetch-input/patches/setup-offline-binaries.sh`:
- Around line 107-110: Replace the PID-based temporary directory creation in the
extraction flow with mktemp -d, assigning its generated path to tmp_dir and
preserving the existing cleanup and mkdir behavior as appropriate. Ensure the
directory is created uniquely and unpredictably before extracting the VSIX.
In `@Makefile`:
- Around line 106-163: Extract the duplicated awk-based conf parsing and
PARSE_FAILED handling from build_image and build_hermetic_image into a shared
parse_build_args_conf macro. Update both image-build macros to call it and reuse
its generated build-args value, preserving the current validation, quoting, and
error behavior.
In `@scripts/lockfile-generators/helpers/patch-rh-wheel-only-packages.py`:
- Line 216: Update the unpacking assignment in the wheel-processing logic around
parse_wheels_section to prefix the unused ref_header variable with an
underscore, preserving the existing use of ref_wheel_lines and behavior.
In `@scripts/lockfile-generators/README.md`:
- Around line 42-51: Update the README sections describing the Python lockfiles
and codeserver pip strategy to match the rhoai-3.3 branch configuration, where
PUBLIC_INDEX_PROJECTS is empty and create-requirements-lockfile.sh uses RH index
mode. Retitle or clearly label any rhoai-2.25-specific guidance as
reference-only, and ensure the documented commands, projects, and lockfile paths
do not imply codeserver uses public PyPI on this branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 948d1c91-f48c-4d19-9446-dbd44c657a2b
⛔ Files ignored due to path filters (8)
codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/custom-packages/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/vite/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/emmet/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/microsoft-authentication/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/remote/package-lock.jsonis excluded by!**/package-lock.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/test/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (106)
.github/actions/apt-install/action.yml.github/actions/capture-kernel-logs/action.yml.github/actions/install-podman-action/action.yml.github/actions/playwright-test/action.yml.github/workflows/build-notebooks-TEMPLATE.yaml.github/workflows/build-notebooks-pr-aipcc.yaml.github/workflows/build-notebooks-pr-rhel.yaml.github/workflows/build-notebooks-pr.yaml.github/workflows/build-notebooks-push.yaml.github/workflows/code-quality.yaml.github/workflows/test-install-podman.yaml.gitmodules.tekton/odh-workbench-codeserver-datascience-cpu-py312-pull-request.yaml.tekton/odh-workbench-codeserver-datascience-cpu-py312-v3-3-push.yaml.tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-v3-3-push.yamlMakefileci/cached-builds/containers.confci/cached-builds/podman.serviceci/cached-builds/podman.socketcodeserver/Extensions.mdcodeserver/README.mdcodeserver/ubi9-python-3.12/Dockerfile.cpucodeserver/ubi9-python-3.12/Dockerfile.konflux.cpucodeserver/ubi9-python-3.12/README.mdcodeserver/ubi9-python-3.12/build-args/cpu.confcodeserver/ubi9-python-3.12/build-args/konflux.cpu.confcodeserver/ubi9-python-3.12/nginx/httpconf/http.confcodeserver/ubi9-python-3.12/nginx/root/opt/app-root/nginxconf.sedcodeserver/ubi9-python-3.12/nginx/serverconf/proxy.conf.templatecodeserver/ubi9-python-3.12/nginx/serverconf/proxy.conf.template_nbprefixcodeserver/ubi9-python-3.12/prefetch-input/code-servercodeserver/ubi9-python-3.12/prefetch-input/odh/artifacts.in.yamlcodeserver/ubi9-python-3.12/prefetch-input/odh/artifacts.lock.yamlcodeserver/ubi9-python-3.12/prefetch-input/odh/rpms.in.yamlcodeserver/ubi9-python-3.12/prefetch-input/odh/rpms.lock.yamlcodeserver/ubi9-python-3.12/prefetch-input/patches/README.mdcodeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.shcodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/README.mdcodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/agent-browser/postinstall.jscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/ci/build/build-vscode.shcodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/ci/dev/postinstall.shcodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/custom-packages/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/gulpfile.reh.tscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/lib/extensions.tscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/lib/tsgo.tscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/npm/preinstall.tscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/vite/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/emmet/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/microsoft-authentication/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/remote/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/ripgrep/postinstall.jscodeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/test/package.jsoncodeserver/ubi9-python-3.12/prefetch-input/patches/codeserver-offline-env.shcodeserver/ubi9-python-3.12/prefetch-input/patches/copy-gha-native-bindings.shcodeserver/ubi9-python-3.12/prefetch-input/patches/setup-offline-binaries.shcodeserver/ubi9-python-3.12/prefetch-input/patches/tweak-gha.shcodeserver/ubi9-python-3.12/prefetch-input/repos/centos.repocodeserver/ubi9-python-3.12/prefetch-input/repos/epel.repocodeserver/ubi9-python-3.12/prefetch-input/repos/openshift-clients.repocodeserver/ubi9-python-3.12/prefetch-input/repos/rhsm-pulp.repocodeserver/ubi9-python-3.12/prefetch-input/repos/ubi.repocodeserver/ubi9-python-3.12/prefetch-input/rhds/artifacts.in.yamlcodeserver/ubi9-python-3.12/prefetch-input/rhds/artifacts.lock.yamlcodeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.in.yamlcodeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.lock.yamlcodeserver/ubi9-python-3.12/pylock.tomlcodeserver/ubi9-python-3.12/pyproject.tomlcodeserver/ubi9-python-3.12/requirements.cpu.txtcodeserver/ubi9-python-3.12/run-code-server.shcodeserver/ubi9-python-3.12/run-nginx.shcodeserver/ubi9-python-3.12/s390x.patchcodeserver/ubi9-python-3.12/utils/ms-python.python-2026.4.0.vsixcodeserver/ubi9-python-3.12/utils/ms-toolsai.jupyter-2025.9.1.vsixcodeserver/ubi9-python-3.12/utils/ms-vscode.js-debug-companion.1.1.3.vsixcodeserver/ubi9-python-3.12/utils/ms-vscode.js-debug.1.112.0.vsixcodeserver/ubi9-python-3.12/utils/ms-vscode.vscode-js-profile-table.1.0.10.vsixcodeserver/ubi9-python-3.12/utils/process.shcodeserver/ubi9-python-3.12/uv.lock.d/pylock.cpu.tomlcodeserver/ubi9-python-3.12/uv.lock.d/rh-wheel-only.ref.tomlscripts/buildinputs/buildinputs_test.goscripts/check-payload/config.tomlscripts/index_url_resolver.pyscripts/lockfile-generators/Dockerfile.rpm-lockfilescripts/lockfile-generators/README.mdscripts/lockfile-generators/create-artifact-lockfile.pyscripts/lockfile-generators/create-go-lockfile.shscripts/lockfile-generators/create-requirements-lockfile.shscripts/lockfile-generators/create-rpm-lockfile.shscripts/lockfile-generators/download-npm.shscripts/lockfile-generators/helpers/download-pip-packages.pyscripts/lockfile-generators/helpers/hermeto-common.shscripts/lockfile-generators/helpers/hermeto-fetch-gomod.shscripts/lockfile-generators/helpers/hermeto-fetch-npm.shscripts/lockfile-generators/helpers/hermeto-fetch-rpm.shscripts/lockfile-generators/helpers/patch-rh-wheel-only-packages.pyscripts/lockfile-generators/helpers/pylock-to-requirements.pyscripts/lockfile-generators/helpers/rpm-lockfile-generate.shscripts/lockfile-generators/patches/apply-patches.shscripts/lockfile-generators/patches/rpm-lockfile-prototype-dnf-conf.patchscripts/lockfile-generators/prefetch-all.shscripts/lockfile-generators/rewrite-npm-urls.shscripts/pylocks_generator.pytests/browser/tests/codeserver.spec.tstests/test_main.py
💤 Files with no reviewable changes (1)
- codeserver/ubi9-python-3.12/Dockerfile.cpu
🚧 Files skipped from review as they are similar to previous changes (56)
- codeserver/ubi9-python-3.12/prefetch-input/rhds/artifacts.lock.yaml
- codeserver/ubi9-python-3.12/nginx/httpconf/http.conf
- codeserver/ubi9-python-3.12/prefetch-input/odh/artifacts.in.yaml
- codeserver/ubi9-python-3.12/prefetch-input/repos/openshift-clients.repo
- codeserver/ubi9-python-3.12/utils/ms-toolsai.jupyter-2025.9.1.vsix
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/test/package.json
- ci/cached-builds/podman.socket
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/package.json
- scripts/lockfile-generators/patches/apply-patches.sh
- codeserver/ubi9-python-3.12/utils/ms-vscode.vscode-js-profile-table.1.0.10.vsix
- .gitmodules
- codeserver/ubi9-python-3.12/nginx/root/opt/app-root/nginxconf.sed
- scripts/lockfile-generators/helpers/hermeto-common.sh
- codeserver/ubi9-python-3.12/utils/ms-vscode.js-debug-companion.1.1.3.vsix
- codeserver/ubi9-python-3.12/prefetch-input/code-server
- codeserver/ubi9-python-3.12/uv.lock.d/rh-wheel-only.ref.toml
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/remote/package.json
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/custom-packages/package.json
- codeserver/ubi9-python-3.12/prefetch-input/repos/epel.repo
- ci/cached-builds/podman.service
- codeserver/ubi9-python-3.12/build-args/cpu.conf
- codeserver/ubi9-python-3.12/prefetch-input/odh/rpms.in.yaml
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/vite/package.json
- ci/cached-builds/containers.conf
- codeserver/ubi9-python-3.12/prefetch-input/odh/artifacts.lock.yaml
- scripts/lockfile-generators/Dockerfile.rpm-lockfile
- codeserver/ubi9-python-3.12/build-args/konflux.cpu.conf
- .github/actions/apt-install/action.yml
- codeserver/ubi9-python-3.12/utils/ms-python.python-2026.4.0.vsix
- codeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.in.yaml
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/agent-browser/postinstall.js
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/microsoft-authentication/package.json
- codeserver/ubi9-python-3.12/prefetch-input/repos/ubi.repo
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/README.md
- codeserver/ubi9-python-3.12/nginx/serverconf/proxy.conf.template_nbprefix
- codeserver/ubi9-python-3.12/utils/process.sh
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/extensions/emmet/package.json
- scripts/lockfile-generators/patches/rpm-lockfile-prototype-dnf-conf.patch
- .github/actions/capture-kernel-logs/action.yml
- codeserver/ubi9-python-3.12/utils/ms-vscode.js-debug.1.112.0.vsix
- codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/package.json
- .github/actions/playwright-test/action.yml
- codeserver/ubi9-python-3.12/nginx/serverconf/proxy.conf.template
- .github/workflows/code-quality.yaml
- scripts/buildinputs/buildinputs_test.go
- codeserver/ubi9-python-3.12/run-code-server.sh
- .github/actions/install-podman-action/action.yml
- .tekton/odh-workbench-codeserver-datascience-cpu-py312-pull-request.yaml
- codeserver/ubi9-python-3.12/s390x.patch
- codeserver/ubi9-python-3.12/prefetch-input/patches/tweak-gha.sh
- codeserver/ubi9-python-3.12/requirements.cpu.txt
- .tekton/odh-workbench-codeserver-datascience-cpu-py312-v3-3-push.yaml
- codeserver/ubi9-python-3.12/Dockerfile.konflux.cpu
- scripts/lockfile-generators/rewrite-npm-urls.sh
- codeserver/ubi9-python-3.12/prefetch-input/repos/rhsm-pulp.repo
- codeserver/ubi9-python-3.12/prefetch-input/rhds/artifacts.in.yaml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (15)
.github/workflows/build-notebooks-pr.yaml (1)
76-84: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' "## .github/AGENTS.md" if [ -f .github/AGENTS.md ]; then cat -n .github/AGENTS.md else echo "missing" fi printf '\n%s\n' "## workflow outline" ast-grep outline .github/workflows/build-notebooks-pr.yaml printf '\n%s\n' "## relevant workflow slice" sed -n '1,180p' .github/workflows/build-notebooks-pr.yaml printf '\n%s\n' "## search for git diff / fetch / pipefail usage" rg -n "git diff --name-only|pipefail|fetch-depth|origin/\\$\\{BASE_REF\\}|HEAD_REF|fork-check|Annotate PR for fork guidance" .github/workflows/build-notebooks-pr.yaml .github/AGENTS.mdRepository: red-hat-data-services/notebooks
Length of output: 8638
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' "## workflow shell defaults and fetch context" sed -n '1,120p' .github/workflows/build-notebooks-pr.yaml | nl -ba printf '\n%s\n' "## git shallow-diff behavior probe" tmpdir="$(mktemp -d)" cd "$tmpdir" git init -q git config user.email test@example.com git config user.name tester echo base > f git add f git commit -q -m base git branch base git checkout -q -b head echo head >> f git commit -q -am head git checkout -q base git checkout -q --orphan shallow git rm -q -rf . || true echo shallow > g git add g git commit -q -m shallow # create a shallow-like situation by cloning with limited history cd "$tmpdir" git clone -q --depth 1 "file://$tmpdir" clone cd clone printf 'merge-base present? ' if git merge-base base head >/dev/null 2>&1; then echo yes; else echo no; fi printf 'diff exit status: ' set +e git diff --name-only "base...head" >/tmp/diff.out 2>/tmp/diff.err status=$? set -e printf '%s\n' "$status" printf 'stderr: ' cat /tmp/diff.errRepository: red-hat-data-services/notebooks
Length of output: 254
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' "## workflow excerpt" sed -n '52,86p' .github/workflows/build-notebooks-pr.yaml printf '\n%s\n' "## shallow git probe" tmpdir="$(mktemp -d)" cd "$tmpdir" git init -q repo cd repo git config user.email test@example.com git config user.name tester echo base > f git add f git commit -q -m base git branch base git checkout -q -b head echo head >> f git commit -q -am head # Make a shallow clone that knows only the tip commits. cd "$tmpdir" git clone -q --depth 1 "file://$tmpdir/repo" shallow cd shallow # Add refs matching the workflow's expected names. git fetch -q --depth 1 origin base:refs/remotes/origin/base git fetch -q --depth 1 origin head:head printf 'merge-base present: ' if git merge-base "origin/base" "head" >/dev/null 2>&1; then echo yes else echo no fi set +e git diff --name-only "origin/base...head" >/tmp/diff.out 2>/tmp/diff.err status=$? set -e printf 'git diff exit: %s\n' "$status" printf 'stderr: ' cat /tmp/diff.err printf 'stdout: ' cat /tmp/diff.outRepository: red-hat-data-services/notebooks
Length of output: 2254
🏁 Script executed:
#!/bin/bash set -euo pipefail tmpdir="$(mktemp -d)" cd "$tmpdir" git init -q --bare origin.git git clone -q origin.git work cd work git config user.email test@example.com git config user.name tester # Create a base branch with a few commits. echo a > f git add f git commit -q -m a git branch -M main git push -q origin main echo b >> f git commit -q -am b git push -q origin main # Create a feature branch from the first commit. first_commit="$(git rev-list --max-parents=0 HEAD)" git checkout -q -b feature "$first_commit" echo feature > g git add g git commit -q -m feature git push -q origin feature cd "$tmpdir" git clone -q --depth 1 file://"$tmpdir/origin.git" shallow cd shallow git fetch -q --depth 1 origin main:refs/remotes/origin/main git fetch -q --depth 1 origin feature:refs/remotes/origin/feature printf 'commits fetched: ' git rev-list --all --count printf 'merge-base: ' set +e mb="$(git merge-base origin/main origin/feature 2>/dev/null)" mb_status=$? set -e printf 'status=%s value=%s\n' "$mb_status" "${mb:-<none>}" set +e git diff --name-only "origin/main...origin/feature" >/tmp/out 2>/tmp/err status=$? set -e printf 'git diff exit=%s\n' "$status" printf 'stderr:\n' cat /tmp/err printf 'stdout:\n' cat /tmp/outRepository: red-hat-data-services/notebooks
Length of output: 508
🏁 Script executed:
#!/bin/bash set -euo pipefail tmpdir="$(mktemp -d)" cd "$tmpdir" git init -q --bare origin.git git clone -q origin.git work cd work git config user.email test@example.com git config user.name tester # main: A -> B echo A > f git add f git commit -q -m A git branch -M main git push -q origin main echo B >> f git commit -q -am B git push -q origin main # feature from A a_commit="$(git rev-list --max-parents=0 HEAD)" git checkout -q -b feature "$a_commit" echo C > g git add g git commit -q -m C git push -q origin feature printf 'origin refs:\n' git --git-dir="$tmpdir/origin.git" show-ref cd "$tmpdir" git clone -q --depth 1 --branch main file://"$tmpdir/origin.git" shallow cd shallow printf '\nlocal refs:\n' git show-ref || true git fetch -q origin feature:refs/remotes/origin/feature printf '\nmerge-base status: ' set +e git merge-base origin/main origin/feature >/tmp/mb.out 2>/tmp/mb.err mb_status=$? set -e printf '%s\n' "$mb_status" printf 'merge-base stderr: ' cat /tmp/mb.err printf 'merge-base stdout: ' cat /tmp/mb.out printf '\ngit diff status: ' set +e git diff --name-only "origin/main...origin/feature" >/tmp/diff.out 2>/tmp/diff.err diff_status=$? set -e printf '%s\n' "$diff_status" printf 'git diff stderr: ' cat /tmp/diff.err printf 'git diff stdout: ' cat /tmp/diff.outRepository: red-hat-data-services/notebooks
Length of output: 362
🏁 Script executed:
#!/bin/bash set -euo pipefail tmpdir="$(mktemp -d)" cd "$tmpdir" git init -q --bare origin.git git clone -q origin.git work cd work git config user.email test@example.com git config user.name tester printf 'step 1: initial commit\n' echo A > f git add f git commit -q -m A git branch -M main git push -q origin main printf 'origin after main push:\n' git --git-dir="$tmpdir/origin.git" show-ref printf '\nstep 2: second main commit\n' echo B >> f git commit -q -am B git push -q origin main printf 'origin after second main push:\n' git --git-dir="$tmpdir/origin.git" show-ref printf '\nstep 3: feature branch from first commit\n' first_commit="$(git rev-list --max-parents=0 HEAD)" git checkout -q -b feature "$first_commit" echo C > g git add g git commit -q -m C git push -q origin feature printf 'origin after feature push:\n' git --git-dir="$tmpdir/origin.git" show-ref printf '\nstep 4: shallow clone and diff\n' cd "$tmpdir" git clone -q --depth 1 --branch main file://"$tmpdir/origin.git" shallow cd shallow git fetch -q origin feature:refs/remotes/origin/feature printf 'local refs:\n' git show-ref printf '\nmerge-base status: ' set +e git merge-base origin/main origin/feature >/tmp/mb.out 2>/tmp/mb.err mb_status=$? set -e printf '%s\n' "$mb_status" printf 'merge-base stderr: ' cat /tmp/mb.err printf 'merge-base stdout: ' cat /tmp/mb.out printf '\ngit diff status: ' set +e git diff --name-only "origin/main...origin/feature" >/tmp/diff.out 2>/tmp/diff.err diff_status=$? set -e printf '%s\n' "$diff_status" printf 'git diff stderr: ' cat /tmp/diff.err printf 'git diff stdout: ' cat /tmp/diff.outRepository: red-hat-data-services/notebooks
Length of output: 396
Guard the annotation step against shallow-history diff failures.
FIRST_FILE:-CONTRIBUTING.mdonly covers an empty diff. Ifgit diffcan’t resolve the 3-dot range in this shallow checkout, the pipeline fails under the default bash-e -o pipefailsettings and aborts the fork-PR job just to emit a warning.🛡️ Proposed fix
- FIRST_FILE=$(git diff --name-only "origin/${BASE_REF}...${HEAD_REF}" | head -1) + FIRST_FILE=$(git diff --name-only "origin/${BASE_REF}...${HEAD_REF}" 2>/dev/null | head -1 || true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.- name: Annotate PR for fork guidance if: steps.fork-check.outputs.is_fork == 'true' run: | FIRST_FILE=$(git diff --name-only "origin/${BASE_REF}...${HEAD_REF}" 2>/dev/null | head -1 || true) FIRST_FILE="${FIRST_FILE:-CONTRIBUTING.md}" echo "::warning file=${FIRST_FILE},line=1,title=Fork PR — subscription builds skipped::Push your branch to the main repo for full CI. See CONTRIBUTING.md." env: HEAD_REF: ${{ github.event.pull_request.head.ref }} BASE_REF: ${{ github.event.pull_request.base.ref }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-notebooks-pr.yaml around lines 76 - 84, Update the “Annotate PR for fork guidance” step so failure to resolve the shallow-checkout 3-dot range does not abort the job. Make the git diff lookup tolerate errors and fall back to CONTRIBUTING.md when it cannot produce a file, while preserving the existing first-file selection and warning annotation behavior..github/workflows/build-notebooks-TEMPLATE.yaml (2)
332-458: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Multiple GitHub Actions template-injection sites in this step (flagged by zizmor).
Three lines here directly interpolate
${{ inputs.target }}intorun:script bodies rather than throughenv:, even though this same step already correctly passesMAKE_TARGET: ${{ inputs.target }}viaenv:(Line 378):
- Line 340:
echo "... for target ${{ inputs.target }}"— redundant and unsafe;$MAKE_TARGETis already available and quoted safely.- Line 391:
run: uv run make ${{ inputs.target }}— the actual build command is built by direct interpolation. This is the most severe instance: a craftedinputs.target(e.g. containing`/$()/;) executes arbitrary shell commands in the step that has GHCR/AIPCC podman auth already on disk from earlier login steps.- Line 455:
run: "ci/cached-builds/has_tests.py --target ${{ inputs.target }}"— same pattern.Since
inputs.targetis sourced from Makefile target names in the PR's own tree (attacker-modifiable in a same-repopull_requesttrigger), this is a real injection vector, not just a style nit. Convert all three to reference anenv:-provided variable, consistent with theMAKE_TARGETpattern already used in this same step.🔒 Proposed fix
- COMPONENT_DIR=$(python3 -c "...os.environ['MAKE_TARGET']...") - DOCKERFILE=$(python3 -c "...os.environ['MAKE_TARGET']...") if [ -z "$COMPONENT_DIR" ]; then - echo "Could not derive COMPONENT_DIR from Makefile for target ${{ inputs.target }}" >&2 + echo "Could not derive COMPONENT_DIR from Makefile for target ${MAKE_TARGET}" >&2 exit 1 fi ... - name: "Build: make ${{ inputs.target }}" id: make-target - run: uv run make ${{ inputs.target }} + run: uv run make "${TARGET}" env: + TARGET: ${{ inputs.target }} IMAGE_TAG: "${{ steps.calculated_vars.outputs.IMAGE_TAG }}" ... - name: "Check if we have tests or not" id: have-tests if: ${{ !cancelled() && (steps.make-target.outcome == 'success') }} - run: "ci/cached-builds/has_tests.py --target ${{ inputs.target }}" + run: ci/cached-builds/has_tests.py --target "${TARGET}" + env: + TARGET: ${{ inputs.target }}🧰 Tools
🪛 zizmor (1.26.1)
[error] 340-340: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 391-391: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 455-455: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-notebooks-TEMPLATE.yaml around lines 332 - 458, Replace all direct `${{ inputs.target }}` interpolations in the shell bodies for the prefetch error message, the “Build: make” step, and the “Check if we have tests or not” step with a quoted environment variable. Reuse the existing MAKE_TARGET environment value where available, and add the same env mapping to the other steps before referencing it in their commands.Source: Linters/SAST tools
480-489: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Same template-injection pattern for the Makefile test runner.
Line 488 interpolates
${{ inputs.target }}directly into therun:command (python3 ci/cached-builds/make_test.py --target ${{ inputs.target }}), the same unsafe pattern flagged above. Please route this throughenv:/$TARGETas well.🧰 Tools
🪛 zizmor (1.26.1)
[error] 488-488: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-notebooks-TEMPLATE.yaml around lines 480 - 489, Update the “Run image tests” step identified by makefile-tests to pass inputs.target through its env block as TARGET, then invoke make_test.py using the shell-safe $TARGET variable instead of directly interpolating the workflow expression in run.Source: Linters/SAST tools
.tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-v3-3-push.yaml (1)
58-68: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not modify
.tekton/files directly.As per coding guidelines, do not modify files in
.tekton/directly in thered-hat-data-services/notebooksrepository. PipelineRuns are synced fromkonflux-central. Please revert these changes here and update them inkonflux-centralinstead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-v3-3-push.yaml around lines 58 - 68, Revert the changes in the `.tekton` pipeline definition, including the `rhel-subscription-activation-key` value, and leave this repository’s generated pipeline file unchanged. Apply the pipeline update in the referenced `konflux-central` repository instead.Source: Coding guidelines
codeserver/Extensions.md (1)
9-9: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Redundant wording: "inside of" → "inside".
Static analysis flags this as redundant phrasing in user-facing docs.
✏️ Proposed fix
-> Detail: some extensions are already available inside of other extensions, that are called "extension packages", i.e., when you install the `ms-python.python` extension, it already comes bundled with the `ms-python.debugpy` extension. +> Detail: some extensions are already available inside other extensions, that are called "extension packages", i.e., when you install the `ms-python.python` extension, it already comes bundled with the `ms-python.debugpy` extension.🧰 Tools
🪛 LanguageTool
[style] ~9-~9: This phrase is redundant. Consider using “inside”.
Context: ...: some extensions are already available inside of other extensions, that are called "exte...(OUTSIDE_OF)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/Extensions.md` at line 9, In the extension package description, replace the redundant phrase “inside of” with “inside” while preserving the surrounding explanation and examples.Source: Linters/SAST tools
codeserver/README.md (1)
11-11: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail git ls-files codeserver/README.md echo "----" nl -ba codeserver/README.md | sed -n '1,120p' echo "----" rg -n "RHOAI 2\.25|AIPCC CPU|rhoai-3\.3|2\.25 notes|prefetch-input/rhds|ubi9/python-312" codeserver/README.mdRepository: red-hat-data-services/notebooks
Length of output: 235
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- codeserver/README.md (first 140 lines) ----" awk '{printf "%4d %s\n", NR, $0}' codeserver/README.md | sed -n '1,140p' echo "---- matches ----" rg -n "RHOAI 2\.25|AIPCC CPU|rhoai-3\.3|2\.25 notes|prefetch-input/rhds|ubi9/python-312" codeserver/README.md || trueRepository: red-hat-data-services/notebooks
Length of output: 3121
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- branch / commit ----" git branch --show-current || true git rev-parse --short HEAD || true echo "---- RHOAI / 3.3 references in code-server ----" rg -n "RHOAI 3\.3|RHOAI 2\.25|AIPCC CPU|2\.25 notes|3\.3" codeserver || true echo "---- top-level README / docs references ----" rg -n "RHOAI 3\.3|RHOAI 2\.25|AIPCC CPU|2\.25 notes|3\.3" README.md .github docs codeserver -g '!**/node_modules/**' || trueRepository: red-hat-data-services/notebooks
Length of output: 50388
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- codeserver/ubi9-python-3.12/README.md (first 80 lines) ----" awk '{printf "%4d %s\n", NR, $0}' codeserver/ubi9-python-3.12/README.md | sed -n '1,80p' echo "---- build-args files ----" awk '{printf "%4d %s\n", NR, $0}' codeserver/ubi9-python-3.12/build-args/cpu.conf | sed -n '1,40p' echo "----" awk '{printf "%4d %s\n", NR, $0}' codeserver/ubi9-python-3.12/build-args/konflux.cpu.conf | sed -n '1,40p'Repository: red-hat-data-services/notebooks
Length of output: 5509
Update the README to match the 3.3 build target.
codeserver/README.md:11,32-44still labelsubi9-python-3.12/as “RHOAI 2.25” and keeps a “RHOAI 2.25 notes” section, butcodeserver/ubi9-python-3.12/build-args/konflux.cpu.confalready targetsRELEASE=3.3with the AIPCC CPU base. Refresh the overview so readers don’t follow stale 2.25 guidance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/README.md` at line 11, Update the codeserver README overview and the “RHOAI 2.25 notes” section to reflect the 3.3 build target and AIPCC CPU base configured in konflux.cpu.conf. Remove or revise stale RHOAI 2.25 labels and guidance while preserving the existing directory overview structure.codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/gulpfile.reh.ts (1)
61-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== target file slice ==" sed -n '1,120p' codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/gulpfile.reh.ts echo echo "== search for ppc64 / s390x / system node refs ==" rg -n "ppc64|s390x|system Node|offline-binaries|prefetch|nodejs\(\)|expectedName|armv7l|ppc64le" \ codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build \ codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode -g '!**/node_modules/**' echo echo "== nearby build script(s) ==" fd -t f "build-vscode.sh|setup-offline-binaries.sh|gulpfile.reh.ts" codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode -x sh -c 'echo "--- {}"; sed -n "1,220p" "{}" | sed -n "1,220p"'Repository: red-hat-data-services/notebooks
Length of output: 11934
🌐 Web query:
Node.js official releases linux ppc64le tarball naming💡 Result:
The official naming convention for Node.js Linux ppc64le (PowerPC 64-bit Little Endian) tarball releases follows a standard format: node-v-linux-ppc64le. [1][2][3]. The components of the filename are: - node: The package name [1]. - v: The specific release version (e.g., v26.5.0) [1]. - linux-ppc64le: The platform and architecture identifier [4][5]. - : The compression format, which is typically.tar.gz or.tar.xz [4][5][1]. For example, a release for version 26.5.0 would be named node-v26.5.0-linux-ppc64le.tar.gz or node-v26.5.0-linux-ppc64le.tar.xz [1][6]. This naming scheme is consistent across the official Node.js distribution site [1][7].
Citations:
- 1: https://nodejs.org/download/release/latest/
- 2: https://nodejs.org/en/download/archive/current
- 3: https://nodejs.github.io/nodejs-ko/articles/2021/02/10/release-v12.20.2/
- 4: https://github.com/nodejs/nodejs-dist-indexer/blob/main/README.md
- 5: https://github.com/nodejs/nodejs-dist-indexer
- 6: https://nodejs.org/dist/latest/
- 7: http://nodejs.org/dist/
Map
ppc64toppc64lein the Node download path. The Linux fallback usesarchverbatim, soppc64would request a non-existentnode-v*-linux-ppc64.*tarball; Node publishes this target aslinux-ppc64le.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/patches/code-server-v4.112.0/lib/vscode/build/gulpfile.reh.ts` around lines 61 - 63, Update the Node download path to translate Linux `ppc64` to `ppc64le` before constructing the archive URL, while leaving the existing `s390x` and other architecture mappings unchanged. Locate the fallback logic that uses the platform and arch values for the Node tarball name.codeserver/ubi9-python-3.12/prefetch-input/patches/setup-offline-binaries.sh (1)
58-65: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify the set -e + command-substitution-assignment failure mode bash -c 'set -euo pipefail; x=$(which definitely-not-a-real-binary-xyz); echo "reached: $x"' echo "exit status: $?"Repository: red-hat-data-services/notebooks
Length of output: 225
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n-- file excerpt --\n' sed -n '52,70p' codeserver/ubi9-python-3.12/prefetch-input/patches/setup-offline-binaries.sh printf '\n-- set -e assignment behavior --\n' bash -c 'set -euo pipefail; x=$(false); echo "reached: ${x:-<empty>}"' >/tmp/sete_assign.out 2>&1 status=$? printf 'exit=%s\n' "$status" cat /tmp/sete_assign.outRepository: red-hat-data-services/notebooks
Length of output: 1374
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' import subprocess, textwrap, json cases = [ 'set -euo pipefail; x=$(false); echo reached', 'set -euo pipefail; x=$(sh -c "exit 7"); echo reached', 'set -euo pipefail; x=$(command -v definitely-not-a-real-binary-xyz); echo reached', ] for script in cases: p = subprocess.run(['bash', '-lc', script], capture_output=True, text=True) print('---') print(script) print('exit', p.returncode) print('stdout', p.stdout.strip()) print('stderr', p.stderr.strip()) PYRepository: red-hat-data-services/notebooks
Length of output: 429
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' import subprocess cases = [ 'set -euo pipefail; x=$(false); echo reached', 'set -euo pipefail; x=$(sh -c "exit 7"); echo reached', 'set -euo pipefail; x=$(command -v definitely-not-a-real-binary-xyz); echo reached', ] for script in cases: p = subprocess.run(['bash', '-lc', script], capture_output=True, text=True) print('---') print(script) print('exit', p.returncode) print('stdout', p.stdout.strip()) print('stderr', p.stderr.strip()) PYRepository: red-hat-data-services/notebooks
Length of output: 429
Keep the ripgrep fallback from exiting early
RIPGREP_BINARY_PATH=$(which rg)still aborts underset -ewhenrgis missing, so the custom error message never runs. Use a non-fatal lookup before the check so the script can print the intended diagnostic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/patches/setup-offline-binaries.sh` around lines 58 - 65, Update the ripgrep lookup in setup-offline-binaries.sh so a missing rg does not terminate the script under set -e. Make the which rg lookup non-fatal before the existing RIPGREP_BINARY_PATH validation, preserving the custom error message and exit behavior.codeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.lock.yaml (1)
4-20: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Inspect the repo input definitions that feed this generated lockfile # to see whether repoid is arch-templated for layered rhocp/RHEL repos. fd rpms.in.yaml cat codeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.in.yaml 2>/dev/null fd -e repo . codeserver/ubi9-python-3.12/prefetch-input/reposRepository: red-hat-data-services/notebooks
Length of output: 4557
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '--- openshift-clients.repo ---\n' cat -n codeserver/ubi9-python-3.12/prefetch-input/repos/openshift-clients.repo printf '\n--- rhsm-pulp.repo (first 220 lines) ---\n' sed -n '1,220p' codeserver/ubi9-python-3.12/prefetch-input/repos/rhsm-pulp.repo | cat -n printf '\n--- repoid occurrences in prefetch-input ---\n' rg -n 'rhocp-4\.16-for-rhel-9-x86_64-rpms|rhel-9-for-x86_64-(appstream|baseos)-rpms|repoid:' codeserver/ubi9-python-3.12/prefetch-input -g '!**/rpms.lock.yaml'Repository: red-hat-data-services/notebooks
Length of output: 260
🏁 Script executed:
#!/bin/bash set -euo pipefail for f in \ codeserver/ubi9-python-3.12/prefetch-input/repos/openshift-clients.repo \ codeserver/ubi9-python-3.12/prefetch-input/repos/rhsm-pulp.repo do echo "=== $f ===" nl -ba "$f" | sed -n '1,240p' echo done echo '=== matching repoid strings under prefetch-input (excluding lockfile) ===' rg -n 'rhocp-4\.16-for-rhel-9-x86_64-rpms|rhel-9-for-x86_64-(appstream|baseos)-rpms|aarch64|ppc64le|s390x' codeserver/ubi9-python-3.12/prefetch-input -g '!**/rpms.lock.yaml'Repository: red-hat-data-services/notebooks
Length of output: 289
🌐 Web query:
Red Hat RHEL 9 repository IDs aarch64 ppc64le s390x x86_64 baseos appstream rhocp layered repo naming💡 Result:
In Red Hat Enterprise Linux (RHEL) 9, repository IDs follow a structured, predictable naming convention that includes the release version, architecture, and content set [1][2]. Core RHEL 9 Repository Naming Convention The standard naming format for core RHEL 9 repositories is: rhel-9-for---rpms [1][2] Components of the ID: 1. Version: rhel-9 [1][2] 2. Architecture: Supported architectures include x86_64, aarch64, ppc64le, and s390x [3]. 3. Content Set: Typically baseos or appstream [1][2]. Examples of standard repository IDs: - BaseOS (x86_64): rhel-9-for-x86_64-baseos-rpms [1][2] - AppStream (x86_64): rhel-9-for-x86_64-appstream-rpms [1][2] - BaseOS (aarch64): rhel-9-for-aarch64-baseos-rpms [3] - AppStream (ppc64le): rhel-9-for-ppc64le-appstream-rpms [3] - BaseOS (s390x): rhel-9-for-s390x-baseos-rpms [3] Red Hat OpenShift (RHOCP) and Layered Repositories Repositories for Red Hat OpenShift Container Platform (RHOCP) and other layered products do not always follow the same rigid structure as core RHEL repositories. They are often defined in manifest files (such as those used for Red Hat CoreOS) and frequently include specific product versions [4][5][6]. Common patterns for these repositories include: - RHOCP: Often appear in formats like rhel--server-ose- (e.g., rhel-9.6-server-ose-4.19) [4][5]. - Other Layered Content: May include identifiers like fast-datapath or early-kernel [4][7]. To verify the exact repository IDs enabled and available on a specific RHEL 9 system, use the following command: subscription-manager repos --list [2] This will output the specific IDs available for your system architecture and subscription [2]. You can also inspect the repository configuration files located in /etc/yum.repos.d/ [8].
Citations:
- 1: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/html/upgrading_from_rhel_8_to_rhel_9/appendix_rhel-9-repositories_upgrading-from-rhel-8-to-rhel-9
- 2: https://refs.rackn.io/stable/resources/objects/params/redhat-subscription-repos-list/
- 3: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/html/upgrading_from_rhel_8_to_rhel_9/appendix-rhel-8-repositories_upgrading-from-rhel-8-to-rhel-9
- 4: https://github.com/openshift/os/blob/90085a537331877b78e9ccce73b61e3852e456ea/packages-openshift.yaml
- 5: https://github.com/openshift/os/blob/5e027de8321d4295c13987487bd2fb60cd57b42b/manifest-rhel-9.2.yaml
- 6: https://github.com/openshift/os/blob/ec62070a0f9ea9ffa9924d6279324e35f34c746a/manifest-rhel-9.4.yaml
- 7: https://github.com/openshift/os/blob/49a3e234f2d0381d3a309f10fc29043fcd63592e/packages-openshift.yaml
- 8: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/html/managing_software_with_the_dnf_tool/assembly_searching-for-rhel-9-content_managing-software-with-the-dnf-tool
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,240p' codeserver/ubi9-python-3.12/prefetch-input/repos/openshift-clients.repo printf '\n=== rhsm-pulp.repo ===\n' sed -n '1,260p' codeserver/ubi9-python-3.12/prefetch-input/repos/rhsm-pulp.repo printf '\n=== repoid references in prefetch-input (excluding lockfiles) ===\n' rg -n 'repoid:|rhocp-4\.16-for-rhel-9-x86_64-rpms|rhel-9-for-x86_64-(appstream|baseos)-rpms' codeserver/ubi9-python-3.12/prefetch-input -g '!**/rpms.lock.yaml'Repository: red-hat-data-services/notebooks
Length of output: 2546
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '=== lockfile repoid samples ===' rg -n 'repoid:' codeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.lock.yaml | sed -n '1,40p' echo echo '=== input manifest package list around openshift-clients ===' rg -n -A3 -B6 'openshift-clients|rhel|rhocp' codeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.in.yamlRepository: red-hat-data-services/notebooks
Length of output: 2871
Align
repoidwith the target arch
The non-x86_64 blocks still record x86_64repoidvalues (rhocp-4.16-for-rhel-9-x86_64-rpms,rhel-9-for-x86_64-…) even though the package URLs are arch-specific. That metadata is misleading for any downstream provenance or validation logic that readsrepoid.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/prefetch-input/rhds/rpms.lock.yaml` around lines 4 - 20, Update the repoid fields in the non-x86_64 package blocks, including the aarch64 entries under arches, to use the repository identifier matching each block’s target architecture instead of x86_64. Preserve the existing package URLs and all other lockfile metadata.codeserver/ubi9-python-3.12/pyproject.toml (1)
33-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does uv pip compile honor tool.uv exclude-dependencies without an explicit --exclude flag?💡 Result:
No,
uv pip compiledoes not automatically honortool.uv.exclude-dependenciesdefined in apyproject.tomlfile [1]. Whileexclude-dependenciesis a valid setting in the[tool.uv]table for project-based operations (likeuv lockoruv sync), it is generally not respected by theuv pipinterface [1][2]. Technical discussions and issue reports from theuvrepository indicate that users often finduv pip compileignores these configurations, requiring the use of explicit command-line flags (such as--exclude) to achieve the same result [1]. Additionally,uv's configuration loading forpipcommands is typically limited to the closestpyproject.tomlin the current working directory, and it does not perform cross-project configuration merging or workspace-wide enforcement forexclude-dependencieswhen executingpipcommands [3][1].Citations:
Pass
--exclude py-spyto theuv pip compilecommand.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/pyproject.toml` around lines 33 - 38, Update the dependency compilation configuration or invocation associated with the pyproject.toml [tool.uv] settings to pass --exclude py-spy to uv pip compile. Ensure py-spy remains excluded during dependency compilation, matching the existing exclude-dependencies configuration.Source: Learnings
codeserver/ubi9-python-3.12/README.md (1)
1-27: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
README still says "RHOAI 2.25" throughout — this PR backports to rhoai-3.3.
Lines 4, 52, 56, 64, 96 (anchor
#python-lockfiles-on-rhoai-225), and 265 all referenceRHOAI 2.25/RELEASE=2.25, and the manual-build table (line 265) still points at the publicregistry.access.redhat.com/ubi9/python-312:latestbase rather than the AIPCC CPU base the PR objective says this backport adopts. Meanwhile the actual scripts in this PR already target 3.3 (e.g.rhelai-3.3repo enablement inrpm-lockfile-generate.sh, and.tekton/*-v3-3-*.yamlpipeline files), so the doc appears to be a stale copy that wasn't updated for the 3.3 backport. Following this README as-is would lead engineers to build against the wrong RELEASE/base image.Also applies to: 28-60, 61-97, 245-270
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/README.md` around lines 1 - 27, Update the README’s release references from RHOAI 2.25 and RELEASE=2.25 to the rhoai-3.3/RHOAI 3.3 values used by the scripts and .tekton pipelines, including the Python lockfiles anchor. In the manual-build instructions, replace the public UBI9 Python base image with the AIPCC CPU base image adopted by this backport, keeping the documented commands and links consistent with 3.3.codeserver/ubi9-python-3.12/uv.lock.d/pylock.cpu.toml (1)
404-406: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Multiple pinned dependencies in this lockfile have known HIGH-severity CVEs with available fixes.
Trivy flags several packages pinned here with vulnerabilities that have patched releases:
cryptography==46.0.7(line 406) — GHSA-537c-gmf6-5ccf (vulnerable bundled OpenSSL), fixed in 48.0.1.urllib3==2.6.3(line 2849) — CVE-2026-44432 (decompression DoS) and CVE-2026-44431 (cross-origin header leak), fixed in 2.7.0.protobuf==6.31.1(line 1878) — CVE-2026-0994 (recursion-depth DoS bypass), fixed in 5.29.6/6.33.5.onnx==1.20.0(line 1604) — multiple HIGH/MEDIUM CVEs (GHSA-q56x-g2fj-4rj6 TOCTOU file read/write, CVE-2026-34445, CVE-2026-28500, CVE-2026-27489, CVE-2026-34446/34447, CVE-2026-44512), fixed in 1.21.0/1.22.0.tornado==6.5.5(line 2790) — CVE-2026-49855 (gzip-bomb DoS), CVE-2026-49853 (cross-origin Authorization header leak), CVE-2026-49854, fixed in 6.5.6+.pyjwt==2.12.1(line 2157) — CVE-2026-48526 (auth bypass via forged JWTs) plus several MEDIUM/LOW CVEs, fixed in 2.13.0.idna==3.11(line 787) — CVE-2026-45409 (DoS via crafted long input), fixed in 3.15.starlette(line 2750) — CVE-2026-54283 (request.form() limits silently ignored, DoS) and CVE-2026-54282, fixed in 1.3.1.virtualenv==20.35.4(line 2937) — CVE-2026-22702 (TOCTOU file-op redirect), fixed in 20.36.1.The generation command in this file's header shows the project already supports excluding CVEs via
--constraints ../../dependencies/cve-constraints.txt. Consider bumping the upstreampyproject.toml/requirements pins (or updating that CVE constraints file) and regenerating this lockfile so these resolve to patched versions.Also applies to: 782-787, 1600-1604, 1875-1878, 2153-2157, 2374-2376, 2747-2750, 2788-2790, 2847-2849, 2935-2937
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeserver/ubi9-python-3.12/uv.lock.d/pylock.cpu.toml` around lines 404 - 406, The lockfile pins multiple vulnerable packages, including cryptography, urllib3, protobuf, onnx, tornado, pyjwt, idna, starlette, and virtualenv. Update the relevant upstream dependency pins or ../../dependencies/cve-constraints.txt to require the stated patched versions, then regenerate the lockfile using its documented command so all resolved entries reflect secure releases.Source: Linters/SAST tools
scripts/lockfile-generators/create-artifact-lockfile.py (1)
56-66: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Checksum mismatch is only a warning — the lockfile still pins the (possibly tampered) actual hash.
process_artifactprints a warning on mismatch (Lines 134-137) but still returnschecksum: f"sha256:{checksum}"using the computed hash rather than the expected one (Line 141), soartifacts.lock.yamlends up recording whatever was actually downloaded — silently defeating the integrity check thechecksumfield exists for. Compounding this,download_file(Lines 56-66) never deletestarget_pathon failure, so a partial/corrupt download can also get reused as a "cache hit" on the next run.For contrast,
download-pip-packages.py'sdownload_onein this same PR correctly deletes the file and fails hard on hash mismatch.🛡️ Proposed fix
try: subprocess.run(cmd, check=True) except FileNotFoundError: - raise RuntimeError("wget not found: please install wget") + raise RuntimeError("wget not found: please install wget") from None except subprocess.CalledProcessError as e: - raise RuntimeError(f"wget failed for {url}: exit code {e.returncode}") + target_path.unlink(missing_ok=True) + raise RuntimeError(f"wget failed for {url}: exit code {e.returncode}") from eif checksum.lower() != expected: - print(f" ⚠ Warning: Checksum mismatch for {filename}", file=sys.stderr) + print(f" ✗ Checksum mismatch for {filename}", file=sys.stderr) print(f" Expected: {expected[:16]}...", file=sys.stderr) print(f" Got: {checksum[:16]}...", file=sys.stderr) + cache_file.unlink(missing_ok=True) + sys.exit(1)Also applies to: 121-143
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 61-61: Command coming from incoming request
Context: subprocess.run(cmd, check=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').(subprocess-from-request)
[error] 61-61: Use of unsanitized data to create processes
Context: subprocess.run(cmd, check=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').(os-system-unsanitized-data)
🪛 Ruff (0.15.21)
[error] 62-62:
subprocesscall: check for execution of untrusted input(S603)
[warning] 64-64: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
[warning] 66-66: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/create-artifact-lockfile.py` around lines 56 - 66, Update process_artifact to fail hard on checksum mismatch and never return or record the computed hash when it differs from the expected checksum; only emit the artifact lock entry after validation succeeds. Update download_file to remove target_path when wget raises FileNotFoundError or CalledProcessError, preventing failed or partial downloads from being reused as cache hits.scripts/lockfile-generators/create-rpm-lockfile.sh (1)
151-159: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash fd 'Dockerfile.rpm-lockfile' --exec cat -n {}Repository: red-hat-data-services/notebooks
Length of output: 2801
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== create-rpm-lockfile.sh excerpt ==" cat -n scripts/lockfile-generators/create-rpm-lockfile.sh | sed -n '130,170p' echo echo "== search for build-arg usage and logging around these vars ==" rg -n --hidden --no-ignore-vcs 'ACTIVATION_KEY|ORG|podman build|CACHE_ARGS' scripts/lockfile-generators/create-rpm-lockfile.sh scripts/lockfile-generators/Dockerfile.rpm-lockfileRepository: red-hat-data-services/notebooks
Length of output: 3925
🌐 Web query:
Podman build-arg values recorded in image history podman history no-trunc ARG secret leak documentation💡 Result:
Using ARG directives to pass sensitive information (such as passwords, tokens, or private keys) to a Podman build is a security risk because the values are recorded in the image's history metadata [1][2][3][4]. Even if the secret is not stored in the final filesystem layers, it remains embedded in the image manifest and can be easily retrieved by anyone with access to the image [1]. To verify this, you can run the following command to view the full build history, including the ARG values used during the build: podman history --no-trunc <image_name> Official documentation explicitly warns against this practice [3][4]: Warning: It is not recommended to use build-time variables for passing secrets like github keys, user credentials etc. Build-time variable values are visible to any user of the image with the podman history command [3][4]. To handle secrets securely during a build, you should use the --secret option, which mounts the secret into the container only during the build process without storing it in the image metadata or layers [5][2]. Example of secure secret usage: 1. Define the secret in your Containerfile using a mount: RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret 2. Build the image by passing the secret via the command line: podman build --secret id=mysecret,src=./secret.txt -t myapp. Using --secret ensures the sensitive data is not persisted in the image's history or layers [5][2].
Citations:
- 1: ansible/ansible-builder#801
- 2: https://oneuptime.com/blog/post/2026-03-18-use-arg-instruction-containerfiles-podman/view
- 3: https://man.archlinux.org/man/Containerfile.5.en
- 4: https://github.com/containers/common/blob/main/docs/Containerfile.5.md
- 5: https://docs.podman.io/en/v5.2.0/markdown/podman-build.1.html
🌐 Web query:
site:docs.podman.io build-arg values image history create history secret exposure podman💡 Result:
Using --build-arg to pass sensitive information like passwords or API keys presents a significant security risk because these values are typically stored as plain text within the image's metadata and can be exposed through the image history [1][2][3]. When you use --build-arg, the argument values are often recorded in the image configuration, meaning anyone with access to the image can view them by running commands such as podman history or docker history [4]. Even if an argument is not explicitly added to the environment variable list, its value may still be captured in the history of the commands used to build the image layers [1][2]. To securely pass secrets during the build process, you should use the --secret feature instead [1][5]. The --secret option allows you to pass sensitive data to the build process in a secure manner where the data is mounted temporarily (e.g., at /run/secrets/id) during the execution of a RUN instruction [1][2]. Crucially, secrets provided via this method are not stored in the final image, are not captured in the image history, and cannot be seen in subsequent build stages [1][5][6]. Key practices for secure builds: - Avoid --build-arg for any sensitive data [1][3]. - Use --secret=id=mysecret,src=path/to/secret to mount secrets only when needed during the build [1][2]. - Inside your Containerfile, access these secrets using the --mount flag: RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret [1][5]. - If you need to further limit visibility into the build process, consider using the --omit-history flag during the build, which prevents the recording of build history information in the resulting image [1][2][7].
Citations:
- 1: https://docs.podman.io/en/v5.7.0/markdown/podman-build.1.html
- 2: https://docs.podman.io/en/v5.4.0/markdown/podman-build.1.html
- 3: https://docs.podman.io/en/v5.2.4/markdown/podman-build.1.html
- 4: https://docs.podman.io/en/v3.4.2/markdown/podman-history.1.html
- 5: https://docs.podman.io/en/latest/markdown/podman-build.unit.5.html
- 6: https://docs.podman.io/en/latest/markdown/podman-secret-create.1.html
- 7: https://docs.podman.io/en/stable/markdown/podman-build.1.html
Pass subscription credentials as secrets, not
--build-arg.
ACTIVATION_KEYandORGare passed intopodman buildinscripts/lockfile-generators/create-rpm-lockfile.shand consumed asARGinscripts/lockfile-generators/Dockerfile.rpm-lockfile, which exposes them in image history. Switch both sides to--secret/RUN --mount=type=secretand read them from/run/secrets/....🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/create-rpm-lockfile.sh` around lines 151 - 159, Update the create-rpm-lockfile podman build and Dockerfile.rpm-lockfile to stop passing ACTIVATION_KEY and ORG as build arguments. Provide both values through named --secret mounts, remove their ARG declarations, and have the relevant build command read credentials from /run/secrets/<name> while preserving the existing build behavior.scripts/lockfile-generators/download-npm.sh (1)
98-112: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash [skip_cloning] bash -c ' url="git+ssh://git@github.com/foo/socket.io.git#abcdef123" if [[ "$url" =~ git\+(ssh://git@|https://)github\.com/([^/]+)/([^.#\"]+)(\.git)?#(.+)$ ]]; then echo "MATCHED (unexpected): repo=${BASH_REMATCH[3]}" else echo "NO MATCH -- confirms the dotted-repo-name gap" fi 'Repository: red-hat-data-services/notebooks
Length of output: 263
🏁 Script executed:
#!/bin/bash set -euo pipefail file="scripts/lockfile-generators/download-npm.sh" echo "== outline ==" ast-grep outline "$file" --view expanded || true echo echo "== relevant lines ==" sed -n '80,145p' "$file" | cat -nRepository: red-hat-data-services/notebooks
Length of output: 3863
Allow dots in GitHub repo names here.
([^.#"]+)rejects valid names likesocket.io, sogit+ssh/git+httpsdependencies with dotted repo names are skipped entirely. Widen the repo capture and strip the optional.gitsuffix after matching.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lockfile-generators/download-npm.sh` around lines 98 - 112, Update the GitHub URL regex in the git+ssh/git+https parsing loop to allow dots in repository names, while still excluding delimiters and quote characters. After extracting the repository in this block, remove the optional trailing .git suffix before constructing the filename and archive URL.
|
/build-codeserver |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/containers/workbenches/jupyterlab/jupyterlab_datascience_test.py`:
- Around line 85-87: Move the “-rstudio-” label check out of the test and into a
dedicated session-scoped fixture, such as in conftest.py, that yields the
datascience image and calls pytest.skip() for matching images; update the test
to consume that fixture instead of duplicating the check. Correct the skip
message to say the image “has” the label and remove the unmatched trailing
quote.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 10480069-7c19-466e-ab53-601ffdfe90b2
📒 Files selected for processing (5)
.github/actions/playwright-test/action.ymltests/containers/base_image_test.pytests/containers/conftest.pytests/containers/utils.pytests/containers/workbenches/jupyterlab/jupyterlab_datascience_test.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/playwright-test/action.yml
| name_label = datascience_image.labels.get("name", "") | ||
| if "-rstudio-" in name_label: | ||
| pytest.skip(f"Image {datascience_image.name} does have -rstudio- in {datascience_image.labels['name']=}'") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move skip logic to a session-scoped fixture.
As per coding guidelines, "Skip session-scoped image fixture automatically based on image labels — do not duplicate skip logic". Checking for -rstudio- and calling pytest.skip() directly inside the test violates this rule.
Please extract this check into a dedicated session-scoped fixture (e.g., in conftest.py) that yields the image while automatically filtering out RStudio.
Additionally, the skip message contains a typographical error (a trailing unmatched single quote) and could be rephrased for clarity (e.g., changing "does have" to "has").
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/containers/workbenches/jupyterlab/jupyterlab_datascience_test.py`
around lines 85 - 87, Move the “-rstudio-” label check out of the test and into
a dedicated session-scoped fixture, such as in conftest.py, that yields the
datascience image and calls pytest.skip() for matching images; update the test
to consume that fixture instead of duplicating the check. Correct the skip
message to say the image “has” the label and remove the unmatched trailing
quote.
Source: Coding guidelines
|
/build-codeserver |
ysok
changed the title
|
/build-codeserver |
| trap 'rm -rf "${workdir}"' EXIT | ||
|
|
||
| fetch_tarball() { | ||
| local name="$1" | ||
| local url="$2" | ||
| local dest="${workdir}/${name}" | ||
| local cached="${GENERIC}/${name}" |
| cp "${cached}" "${dest}" | ||
| echo "Using prefetched ${name}" | ||
| else | ||
| echo "Downloading ${url}" | ||
| curl -fsSL "${url}" -o "${dest}" | ||
| fi | ||
| } | ||
|
|
||
| fetch_tarball "util-macros-${UTIL_MACROS_VERSION}.tar.gz" "${UTIL_URL}" | ||
| fetch_tarball "libxkbfile-${X_KB_FILE_VERSION}.tar.gz" "${XKB_URL}" | ||
|
|
||
| cd "${workdir}" | ||
|
|
||
| tar xf "util-macros-${UTIL_MACROS_VERSION}.tar.gz" | ||
| cd "util-macros-${UTIL_MACROS_VERSION}" | ||
| ./configure --prefix=/usr | ||
| make install -j "${MAX_JOBS}" | ||
|
|
||
| cd "${workdir}" | ||
| tar xf "libxkbfile-${X_KB_FILE_VERSION}.tar.gz" | ||
| cd "libxkbfile-${X_KB_FILE_VERSION}" |
|
/build-codeserver |
2 similar comments
|
/build-codeserver |
|
/build-codeserver |
| cp -f "${RIPGREP_BINARY_PATH}" "${ripgrep_dst}/bin/rg" | ||
| chmod 755 "${ripgrep_dst}/bin/rg" | ||
| echo "Copied RIPGREP_BINARY_PATH into ${ripgrep_dst}/bin/rg" | ||
| else | ||
| echo "ERROR: @vscode/ripgrep bin/rg missing (src=${ripgrep_src}/bin RIPGREP_BINARY_PATH=${RIPGREP_BINARY_PATH:-})" >&2 |
|
/build-codeserver |
1 similar comment
|
/build-codeserver |
Upgrade codeserver to v4.112.0 with Konflux hybrid-hermetic builds (npm prefetch; RPMs/Python at build time) so all arches can build, and port GHA/runtime fixes needed for the 3.3 AIPCC base and RH index. Co-authored-by: Cursor <cursoragent@cursor.com>
f80df35 to
0cb8190
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: daniellutz The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Align codeserver browser tests with main's baked tests/browser image flow so AIPCC PR builds stop failing on the missing version helper and Dockerfile. Co-authored-by: Cursor <cursoragent@cursor.com>
|
New changes are detected. LGTM label has been removed. |
|
/build-codeserver |
4 participants
Summary
Dockerfile.konflux.cpu, prefetch-input tree, lockfile generators).rh-index), subscription RPM locks for Konflux (rhds/), and public UBI locks for local/GHA (odh/).GHA_BUILD, prefetch, Playwright) and addHERMETIC_LOCAL=1Makefile support for local builds.Test plan
gmake test(unit/static + dockerfile alignment)HERMETIC_LOCAL=1 BUILD_ARCH=linux/arm64 PUSH_IMAGES=no gmake codeserver-ubi9-python-3.12odh-workbench-codeserver-datascience-cpu-py312-pull-request.yaml)v3-3-pushPipelineRun (follow-up)Made with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Documentation