feat: UI redesign, GitHub Actions CI, security hardening#7
feat: UI redesign, GitHub Actions CI, security hardening#7mricharz wants to merge 15 commits intounraid:masterfrom
Conversation
📝 WalkthroughWalkthroughAdds two GitHub Actions workflows (build and auto-update), a build-matrix config, safer package cleanup and a new Changes
Sequence Diagram(s)sequenceDiagram
actor Scheduler as Cron/Manual
participant AutoWF as Auto-update Workflow
participant NVIDIA as NVIDIA Sites
participant GH as GitHub API
participant GHCR as GHCR
participant JQ as jq
participant Git as Git
Scheduler->>AutoWF: trigger (schedule or manual)
AutoWF->>NVIDIA: fetch driver streams (prod/newfeature/beta/legacy)
AutoWF->>GH: query Unraid kernel releases & runtime repos
AutoWF->>GHCR: query ghcr tags for gcc/runtime images
AutoWF->>JQ: update build-matrix.json / versions.json (rotate, dedupe)
AutoWF->>Git: commit & push if changed
AutoWF->>GH: trigger build workflow when commit made
sequenceDiagram
participant User
participant GH as GitHub Actions
participant Prepare as Prepare Job
participant Build as Build Job (matrix)
participant Container as Container Image
participant Release as GitHub Release
User->>GH: dispatch build workflow
GH->>Prepare: run prepare job (load matrix, output gcc_tag)
GH->>Build: spawn matrix jobs fromJson(matrix)
Build->>Release: inspect kernel release for existing `.txz`
alt `.txz` exists
Build->>Build: mark entry skipped (bypass build/verify/upload)
else not exists
Build->>Container: run containerized build (download kernel, prepare headers, compile)
Container-->>Build: produce `.txz` and `.md5`
Build->>Build: verify checksums and artifact presence
Build->>Release: create/update release and upload assets
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
source/usr/local/emhttp/plugins/nvidia-driver/include/exec.sh (1)
79-81: Redundantsort -Vfor single-value beta.current field.The
get_beta()function appliessort -Vto.branches.beta.current, but this is a single version string (not an array likeproduction[]ornewfeature[]). The sort operation is harmless but unnecessary.This is consistent with how
update-check.shanddownload.shhandle beta - they also treat it as a single value. The redundant sort doesn't cause issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/usr/local/emhttp/plugins/nvidia-driver/include/exec.sh` around lines 79 - 81, The get_beta() pipeline unnecessarily runs sort -V on a single-version value from jq '.branches.beta.current'; remove the redundant sort -V invocation in the command substitution inside function get_beta so it reads the single version string directly (keep the surrounding jq, awk and comm formatting intact), i.e., locate the get_beta() function and delete the "| sort -V" portion applied to the $(cat /tmp/nvidia_branches | jq -r '.branches.beta.current' ...) fragment to simplify the pipeline..github/workflows/build-nvidia.yml (2)
133-136: wget wrapper only strips first occurrence of --show-progress.The bash parameter expansion
${@/--show-progress/}only removes the first occurrence. If wget is called with--show-progressmultiple times (unlikely but possible), subsequent occurrences would remain and could cause issues in the container environment.Use global replacement pattern
- printf "#!/bin/bash\nexec ${real_wget} \"\${`@/--show-progress/`}\"" > /usr/local/bin/wget + printf "#!/bin/bash\nexec ${real_wget} \"\${`@//--show-progress/`}\"" > /usr/local/bin/wgetNote the double slash
//for global replacement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-nvidia.yml around lines 133 - 136, The wget wrapper currently uses the bash parameter expansion `${`@/--show-progress/`}` which only removes the first `--show-progress`; update the wrapper to use the global replacement form `${`@//--show-progress/`}` so all occurrences are stripped before calling the real_wget (adjust the exec call that invokes real_wget to use `"${`@//--show-progress/`}"` and keep the existing chmod/placement logic).
138-139: Potential issue with dummy argument when building proprietary modules.When
MODULE_TYPEis notopensource,BUILD_ARGis empty, and line 138 passes"dummy"tocompile.shsource command. However, line 139 passes empty${BUILD_ARG}tonvidia_driver. This asymmetry may cause issues:
- The
sourcecommand on line 138 receives"dummy"as$2, but this only affects the initial sourcing context- The
nvidia_drivercall on line 139 receives an empty second argument when proprietaryLooking at the relevant snippet from
compile.sh:19-35, the function checksif [ "${2}" == "opensource" ]and falls through to proprietary logic otherwise, so an empty$2should work. However, the source line's dummy argument serves no purpose.Suggested simplification
- source source/compile.sh "${DRIVER_VERSION}" ${BUILD_ARG:-"dummy"} - nvidia_driver "${DRIVER_VERSION}" ${BUILD_ARG} + source source/compile.sh + nvidia_driver "${DRIVER_VERSION}" ${BUILD_ARG}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-nvidia.yml around lines 138 - 139, The source invocation passes a hardcoded "dummy" as the second arg while nvidia_driver uses ${BUILD_ARG}, causing inconsistency; remove the literal "dummy" and make the source call use the same expansion as nvidia_driver (e.g., source source/compile.sh "${DRIVER_VERSION}" "${BUILD_ARG:-}" or simply "${BUILD_ARG}") so both callers (the source of compile.sh and the nvidia_driver function) receive the same second parameter and the compile.sh check (if [ "${2}" == "opensource" ]) behaves consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/build-nvidia.yml:
- Around line 133-136: The wget wrapper currently uses the bash parameter
expansion `${`@/--show-progress/`}` which only removes the first
`--show-progress`; update the wrapper to use the global replacement form
`${`@//--show-progress/`}` so all occurrences are stripped before calling the
real_wget (adjust the exec call that invokes real_wget to use
`"${`@//--show-progress/`}"` and keep the existing chmod/placement logic).
- Around line 138-139: The source invocation passes a hardcoded "dummy" as the
second arg while nvidia_driver uses ${BUILD_ARG}, causing inconsistency; remove
the literal "dummy" and make the source call use the same expansion as
nvidia_driver (e.g., source source/compile.sh "${DRIVER_VERSION}"
"${BUILD_ARG:-}" or simply "${BUILD_ARG}") so both callers (the source of
compile.sh and the nvidia_driver function) receive the same second parameter and
the compile.sh check (if [ "${2}" == "opensource" ]) behaves consistently.
In `@source/usr/local/emhttp/plugins/nvidia-driver/include/exec.sh`:
- Around line 79-81: The get_beta() pipeline unnecessarily runs sort -V on a
single-version value from jq '.branches.beta.current'; remove the redundant sort
-V invocation in the command substitution inside function get_beta so it reads
the single version string directly (keep the surrounding jq, awk and comm
formatting intact), i.e., locate the get_beta() function and delete the "| sort
-V" portion applied to the $(cat /tmp/nvidia_branches | jq -r
'.branches.beta.current' ...) fragment to simplify the pipeline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da1e7988-4ebf-44b1-8ddf-bf2b900d6022
📒 Files selected for processing (6)
.github/workflows/build-nvidia.ymlbuild-matrix.jsonsource/usr/local/emhttp/plugins/nvidia-driver/include/download.shsource/usr/local/emhttp/plugins/nvidia-driver/include/exec.shsource/usr/local/emhttp/plugins/nvidia-driver/include/update-check.shsource/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page
mricharz
changed the title
|
Sorry I've overlooked this PR, however @SimonFair already made a PR with changes so that users only can select driver versions which match their GPU in the system.
Please open up a post in the support thread next time. The plugin will be changed in the coming months so that the OpenSource packages will be the default and only the legacy driver (version 580.x) will be the proprietary on.
Can you describe that a bit more in detail? Sure injection would be also be possible but if someone wants to do something malicious to your server and already has access to your server the plugin isn't the first attack vector I think.
How long does the build from the driver take on GitHub Actions?
There shouldn't be much API calls in the first place since the files are cached or do I get the new method wrong? Is that even necessary? One additional question about libnvidia-container: 1.19.0 is still in RC state and RC branches are not auto compiled so the RC version is not available yet for Unraid, I also don't want to ship RC components with this driver at least when it's avoidable. BTW A few minor changes will be made when 1.19.0 is released anyways, but that shouldn't mean anything for that PR. |
Actions took around 7 minuten for 12 artifacts
sure, injection can only happen if someone already has access to the UI. But security consists not only of one big wall. today we combine multiple vulneabilities to get a full security breach.
|
So you are building 12 drivers do I get that right?
I completely get your point but I don't think that this counts as a breach since someone already needs access to the Server, however this is always a big discussion anyways and I have nothing against the changes. Just saying.
Sorry, I haven't fully looked through the routine but seems okay to me.
Just click on Sorry that I've overlooked the issue and also this PR, but somehow GitHub doesn't send me notifications about new Issues anymore. |
- Redesign settings page with card-based layout and full Unraid theme support (dark/light), responsive 2-column grid, GPU arch & CUDA info - Add GitHub Actions CI with JSON-driven build matrix, auto-detection of new driver versions/kernels, and skip-check for existing builds - Add beta branch support to download.sh, update-check.sh, exec.sh - Security: input validation in update_version(), change_update_check() to prevent command injection; function whitelist replacing $@ dispatcher; safe file cleanup preventing mass deletion when variables are empty - Fix: extract FETCH_VERSIONS() to eliminate redundant API call - Fix: $DRIVERS variable scope bug in legacy 580 driver check - Integrate upstream card-support.php for GPU architecture detection
mricharz
force-pushed
the
pr/upstream-improvements
branch
from
cba5b1f to
63a120d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
source/usr/local/emhttp/plugins/nvidia-driver/include/exec.sh (2)
16-24: Consider modernizing arithmetic and quoting variables.The cache logic correctly uses
FETCH_VERSIONS. For improved robustness and modern shell style:🔧 Proposed improvements
if [ -f /tmp/nvidia_driver ]; then FILETIME=$(stat /tmp/nvidia_driver -c %Y) - DIFF=$(expr $CURENTTIME - $FILETIME) - if [ $DIFF -gt $CHK_TIMEOUT ]; then + DIFF=$((CURENTTIME - FILETIME)) + if [ "$DIFF" -gt "$CHK_TIMEOUT" ]; then FETCH_VERSIONS fi else🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/usr/local/emhttp/plugins/nvidia-driver/include/exec.sh` around lines 16 - 24, The branch that checks /tmp/nvidia_driver should use modern POSIX-safe arithmetic and quote variables: replace the expr call with arithmetic expansion (e.g., DIFF=$(( CURENTTIME - FILETIME ))), ensure all variable expansions are quoted when used in tests and commands (e.g., "$FILETIME", "$DIFF", "$CHK_TIMEOUT"), and quote the filename "/tmp/nvidia_driver" in the -f test; keep the FETCH_VERSIONS calls unchanged. Also verify the correct variable name for CURENTTIME (fix typo to CURRENTTIME if applicable) before using it in the arithmetic.
8-15: Good extraction of version fetching logic; consider quoting$DRIVERS.The
FETCH_VERSIONS()function nicely centralizes the API fetch logic and fixes the variable scope issue. However,$DRIVERSshould be double-quoted in the heredoc to prevent word splitting issues with unusual version strings.🔧 Proposed fix
FETCH_VERSIONS() { DRIVERS="$(wget -qO- https://api.github.com/repos/unraid/unraid-nvidia-driver/releases/tags/${KERNEL_V} | jq -r '.assets[].name' | grep -E -v '\.md5$' | sort -V)" - echo -n "$(grep ${PACKAGE} <<< "$DRIVERS" | awk -F "-" '{print $2}' | sort -V | uniq)" > /tmp/nvidia_driver + echo -n "$(grep "${PACKAGE}" <<< "$DRIVERS" | awk -F "-" '{print $2}' | sort -V | uniq)" > /tmp/nvidia_driver echo -n "$(grep nvos <<< "$DRIVERS" | awk -F "-" '{print $2}' | sort -V | uniq)" > /tmp/nvos_driver🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/usr/local/emhttp/plugins/nvidia-driver/include/exec.sh` around lines 8 - 15, In FETCH_VERSIONS ensure you use quoted variables to prevent word-splitting and globbing: change uses of grep ${PACKAGE} <<< "$DRIVERS" and grep nvos <<< "$DRIVERS" to use quoted/safer forms (e.g., grep -- "${PACKAGE}" <<< "$DRIVERS" and grep -- "nvos" <<< "$DRIVERS") and keep the heredoc input as "$DRIVERS"; also quote expansions like "$(awk ...)" outputs where appropriate so all uses of DRIVERS and PACKAGE are protected (refer to FETCH_VERSIONS, DRIVERS, and the grep/awk pipeline).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/auto-update-matrix.yml:
- Around line 166-177: The jq filter is using .branches inside select() while .
is the current .extra_build item, so capture the root object (e.g., using "as
$root") before iterating .extra_builds[] and then compare each item's
.driver_version against $root.branches.production.driver_version,
$root.branches.newfeature.driver_version, and $root.branches.beta.driver_version
(with // "" fallback) so the select actually filters out branch versions; keep
the subsequent .extra_builds = (.extra_builds | unique_by(.driver_version))
dedupe step as-is.
- Around line 32-44: The current fallback only runs when RAW_DATA is empty, so
parsing failures leave RAW_DATA set but PRB/NFB/BETA/LEGACY empty; change the
fallback logic to detect parse failure by checking the extracted variables (PRB,
NFB, BETA, LEGACY) and trigger the fallback fetch when those are empty (e.g., if
all or the primary expected variables like PRB are empty) in addition to
RAW_DATA being empty; implement the fallback fetch block to run when parse
results are missing, reassign RAW_DATA and re-extract PRB/NFB/BETA/LEGACY, and
keep the final guard that errors out only if RAW_DATA remains empty or required
version vars are still empty after the fallback.
---
Nitpick comments:
In `@source/usr/local/emhttp/plugins/nvidia-driver/include/exec.sh`:
- Around line 16-24: The branch that checks /tmp/nvidia_driver should use modern
POSIX-safe arithmetic and quote variables: replace the expr call with arithmetic
expansion (e.g., DIFF=$(( CURENTTIME - FILETIME ))), ensure all variable
expansions are quoted when used in tests and commands (e.g., "$FILETIME",
"$DIFF", "$CHK_TIMEOUT"), and quote the filename "/tmp/nvidia_driver" in the -f
test; keep the FETCH_VERSIONS calls unchanged. Also verify the correct variable
name for CURENTTIME (fix typo to CURRENTTIME if applicable) before using it in
the arithmetic.
- Around line 8-15: In FETCH_VERSIONS ensure you use quoted variables to prevent
word-splitting and globbing: change uses of grep ${PACKAGE} <<< "$DRIVERS" and
grep nvos <<< "$DRIVERS" to use quoted/safer forms (e.g., grep -- "${PACKAGE}"
<<< "$DRIVERS" and grep -- "nvos" <<< "$DRIVERS") and keep the heredoc input as
"$DRIVERS"; also quote expansions like "$(awk ...)" outputs where appropriate so
all uses of DRIVERS and PACKAGE are protected (refer to FETCH_VERSIONS, DRIVERS,
and the grep/awk pipeline).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09e28314-245e-4e01-86a0-2622a4a8fe52
📒 Files selected for processing (7)
.github/workflows/auto-update-matrix.yml.github/workflows/build-nvidia.ymlbuild-matrix.jsonsource/usr/local/emhttp/plugins/nvidia-driver/include/download.shsource/usr/local/emhttp/plugins/nvidia-driver/include/exec.shsource/usr/local/emhttp/plugins/nvidia-driver/include/update-check.shsource/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page
✅ Files skipped from review due to trivial changes (2)
- build-matrix.json
- .github/workflows/build-nvidia.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- source/usr/local/emhttp/plugins/nvidia-driver/include/download.sh
- source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page
When a driver version doesn't support a kernel version, the NVIDIA installer exits with code 8. Instead of failing the entire job, detect this and skip gracefully with a warning annotation.
The NVIDIA installer returns exit code 1 (not 8) when kernel modules fail to build due to kernel incompatibility, because set -euo pipefail overrides the original exit code. Instead of checking exit codes, check if any .txz package was produced — no output + non-zero exit means the driver is incompatible with the kernel.
Different Unraid versions use different kernel branches (e.g. 7.2.x uses 6.12.x, newer versions use 6.17.x or 6.18.x). Now picks the latest patch version per major.minor branch so all active Unraid versions are covered.
- Fix critical bug: select() inside extra_builds iteration had wrong scope — .branches referenced the current item, not root. Use '. as $root' to capture root object before iterating. - Improve fallback: trigger forum fallback also when nvidia.com returns data but parsing fails (all version vars empty).
|
The current version also auto-updates the build matrix with new driver versions, unraid kernel versions and gcc_tag versions and automatically builds them. And this is how the UI looks: |
|
Thanks for the PR @mricharz as said above:
Would it be possible to turn that around? I also made a comment on the forums here: I also mentioned there if a user has more than one card is there an index for users to see? BTW I also just noticed that the GPU UUID is not shown anymore, is that by design or can they somehow display the UUID's? |
- List all GPUs with expandable detail panels showing chip codename, architecture, module support, device ID, and driver recommendations - Detect module support intersection across all GPUs: if all support open-source → auto-select, if conflict (e.g. Volta+Blackwell) → error - Rename labels: Proprietary → Legacy (Proprietary), Open Source is default - Auto-hide module type dropdown when only one option is valid - Use upstream gpu-support.php for architecture detection instead of manual compute_cap parsing
- Remove dead $is_blackwell variable (replaced by $force_opensource) - Add .nv-tag-unknown CSS and fix fallthrough label to 'Unknown' - Separate recommended_driver guard to avoid empty parentheses - Disable Download button when GPU conflict detected
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
.github/workflows/auto-update-matrix.yml (1)
146-168: Copy the branchmodule_typesintoextra_builds.In
.github/workflows/build-nvidia.yml, Lines 14-44, extra builds fan out directly fromextra_builds[].module_types[]. Hardcoding both types here widens support for pinned old versions instead of preserving what the branch declared.♻️ Suggested fix
- .extra_builds += [{"driver_version": $cur_prb, "module_types": ["proprietary", "opensource"]}] + .extra_builds += [{ + "driver_version": $cur_prb, + "module_types": (.branches.production.module_types // ["proprietary", "opensource"]) + }] ... - .extra_builds += [{"driver_version": $cur_nfb, "module_types": ["proprietary", "opensource"]}] + .extra_builds += [{ + "driver_version": $cur_nfb, + "module_types": (.branches.newfeature.module_types // ["proprietary", "opensource"]) + }] ... - .extra_builds += [{"driver_version": $cur_beta, "module_types": ["proprietary", "opensource"]}] + .extra_builds += [{ + "driver_version": $cur_beta, + "module_types": (.branches.beta.module_types // ["proprietary", "opensource"]) + }]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/auto-update-matrix.yml around lines 146 - 168, The extra_builds insertion currently hardcodes module_types to ["proprietary","opensource"] when rotating branches (see the .extra_builds += [...] lines and branches.production.driver_version / branches.newfeature.driver_version / branches.beta.driver_version); change each insertion to copy the module_types from the branch being rotated (e.g. use the branch's .branches.production.module_types, .branches.newfeature.module_types and .branches.beta.module_types respectively) instead of the hardcoded array so the pinned extra_build preserves the original branch's module_types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/auto-update-matrix.yml:
- Around line 241-247: The workflow currently triggers the downstream "Build
NVIDIA Driver Packages" run with a hardcoded ref (--ref master); update the gh
workflow run invocation in the "Trigger build workflow" step to use the current
branch ref (${{ github.ref_name }}) instead of "master" so the downstream build
runs on the same branch where the commit was made (replace the --ref master
argument with --ref ${{ github.ref_name }} in the gh workflow run command).
In `@source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page`:
- Around line 399-401: The three external anchor elements that open in a new tab
(the links with hrefs
"https://forums.unraid.net/topic/98978-plugin-nvidia-driver/",
"https://github.com/unraid/unraid-nvidia-driver", and
"https://www.nvidia.com/en-us/drivers/unix/" which currently use
target="_blank") should include rel="noopener noreferrer" to prevent
reverse-tabnabbing; update those <a> tags to add rel="noopener noreferrer"
alongside the existing target attribute.
- Around line 260-266: The echoed system-derived variables ($installed_v,
$module_type_display, $cuda_v, $kernel_v) are emitted raw into the HTML; update
each echo to output an HTML-escaped version (e.g., wrap values with
htmlspecialchars(..., ENT_QUOTES, 'UTF-8') or equivalent) so
command/file-derived data is safely encoded; ensure you change echoes in the
Driver Version, Kernel Module, CUDA Version (conditional), and Kernel lines to
use the escaping helper and keep the conditional logic for $cuda_v and $gpu_rows
unchanged.
- Around line 342-346: The code currently uses krsort($sorted) which sorts by
array keys and can misorder the pinned versions coming from $eachlines; replace
the key-based sort with a value-based, version-aware sort (e.g., use
usort($sorted, 'version_compare') and then reverse for descending order, or
rsort($sorted, SORT_NATURAL)) so the loop that calls
mk_option(trim($selected_v), $ver, 'v'.$ver, 'data-type="proprietary"') iterates
versions sorted correctly by their version strings rather than by array index.
- Around line 384-386: The label element uses for="updata_check_selected" but
the corresponding select lacks an id, breaking the label-control binding; update
the select element (the <select name="updata_check_selected">) to include
id="updata_check_selected" so the label correctly associates with the control
(no changes needed to mk_option usage).
- Around line 211-239: The filterVersions function currently always hides driver
options based on module_type, removing the prior user-overridable
"guidance-only" behavior; modify filterVersions to first check a user-controlled
override flag (preserve whatever UI element controls GPU-based filtering—e.g., a
checkbox or input that was used previously) and if that flag indicates filtering
is disabled, skip any hiding logic so drv_version shows all options; otherwise
proceed with the existing logic that reads module_type, computes allowed,
toggles option and optgroup display, and updates selectedIndex and
updateAutoUpdate(); also restore similar override handling where module-type
choice is conditionally removed (the code around lines 315-322) so the UI still
allows users to disable GPU-based filtering and pick any version.
- Around line 315-329: The page is allowing actions during conflict by emitting
a hidden module_type and leaving the update controls enabled; change the else
branch that runs when $is_conflict (and when $force_opensource/$force_legacy are
set) to not inject an implicit hidden `#module_type` value and to render the UI as
non-actionable — e.g., remove the hidden input creation and add a
disabled/hidden state for the Update & Download control or server-side gate the
submit action when $is_conflict is true; make these changes around the
conditional that uses $is_conflict, $force_legacy and $force_opensource and
target the "module_type" element and the Update & Download control so no
install/update can proceed while a conflict exists.
---
Nitpick comments:
In @.github/workflows/auto-update-matrix.yml:
- Around line 146-168: The extra_builds insertion currently hardcodes
module_types to ["proprietary","opensource"] when rotating branches (see the
.extra_builds += [...] lines and branches.production.driver_version /
branches.newfeature.driver_version / branches.beta.driver_version); change each
insertion to copy the module_types from the branch being rotated (e.g. use the
branch's .branches.production.module_types, .branches.newfeature.module_types
and .branches.beta.module_types respectively) instead of the hardcoded array so
the pinned extra_build preserves the original branch's module_types.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48ec1f5a-db5a-44ae-929d-b3bb881db628
📒 Files selected for processing (3)
.github/workflows/auto-update-matrix.ymlbuild-matrix.jsonsource/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page
✅ Files skipped from review due to trivial changes (1)
- build-matrix.json
…oprietary_only to has_legacy_only, remove dead comment
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/auto-update-matrix.yml (1)
12-15: Add workflow concurrency control to avoid push/dispatch races.Manual and scheduled runs can overlap and race on commit/push + downstream dispatch. A workflow-level
concurrencygroup would make this deterministic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/auto-update-matrix.yml around lines 12 - 15, Add a workflow-level concurrency block to prevent overlapping runs: at the top-level of the YAML (above jobs) add a concurrency key with a stable group string like "auto-update-matrix-${{ github.ref }}" (or include github.workflow/github.event_name as needed) and set cancel-in-progress: true; this will ensure the detect-and-update job and any manual/scheduled runs do not race on push/dispatch.source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page (1)
344-348: Consider escaping version strings in option labels for defense-in-depth.While the version values (
$latest_v,$latest_prb_v, etc.) come from controlled backend sources and contain version patterns like "535.104.05", applyinghtmlspecialchars()would provide defense-in-depth against any future changes to version string sources.🛡️ Optional defensive escaping
<optgroup label="Follow Branch (auto-update)" data-group="branch"> - <?php echo mk_option($selected_v, 'latest', 'Latest — v'.$latest_v); ?> - <?php if (!empty(trim($latest_prb_v))) echo mk_option($selected_v, 'latest_prb', 'Production — v'.$latest_prb_v); ?> - <?php if (!empty(trim($latest_nfb_v))) echo mk_option($selected_v, 'latest_nfb', 'New Feature — v'.$latest_nfb_v); ?> - <?php if (!empty(trim($latest_beta_v))) echo mk_option($selected_v, 'latest_beta', 'Beta — v'.$latest_beta_v); ?> - <?php if (!empty(trim($latest_nos_v))) echo mk_option($selected_v, 'latest_nos', 'Open Source — v'.$latest_nos_v); ?> + <?php echo mk_option($selected_v, 'latest', 'Latest — v'.htmlspecialchars($latest_v)); ?> + <?php if (!empty(trim($latest_prb_v))) echo mk_option($selected_v, 'latest_prb', 'Production — v'.htmlspecialchars($latest_prb_v)); ?> + <?php if (!empty(trim($latest_nfb_v))) echo mk_option($selected_v, 'latest_nfb', 'New Feature — v'.htmlspecialchars($latest_nfb_v)); ?> + <?php if (!empty(trim($latest_beta_v))) echo mk_option($selected_v, 'latest_beta', 'Beta — v'.htmlspecialchars($latest_beta_v)); ?> + <?php if (!empty(trim($latest_nos_v))) echo mk_option($selected_v, 'latest_nos', 'Open Source — v'.htmlspecialchars($latest_nos_v)); ?>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page` around lines 344 - 348, The option labels currently interpolate version variables ($latest_v, $latest_prb_v, $latest_nfb_v, $latest_beta_v, $latest_nos_v) directly into the mk_option calls; update those label strings to escape the version values using htmlspecialchars (e.g., htmlspecialchars($latest_v, ENT_QUOTES, 'UTF-8')) before concatenation so mk_option receives HTML-escaped labels for defense-in-depth while leaving the variable sources unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/auto-update-matrix.yml:
- Around line 147-166: The rotation logic for branches (variables
$cur_prb/$cur_nfb/$cur_beta and assignments like
.branches.production.driver_version) can add empty or "null" driver_version
entries into .extra_builds; update each rotation condition to also require the
old/current value is non-empty/non-"null" before pushing it: when checking (if
($new_prb != "") and ($cur_prb != $new_prb) ...) also ensure $cur_prb is not
empty or the string "null" (and do the same for $cur_nfb and $cur_beta) so you
only append valid previous driver_version values to .extra_builds and avoid
creating invalid entries.
- Around line 33-45: The workflow currently only fails when RAW_DATA is empty;
update the fallback parsing block to verify that required parsed variables (PRB,
NFB, BETA, and LEGACY) are present after fetching and parsing and fail fast if
any are empty: after the lines that set PRB, NFB, BETA, and LEGACY, add a guard
that echoes an error (e.g., "::error::Missing NVIDIA driver versions: ...") and
exits non-zero when any of PRB/NFB/BETA/LEGACY are empty so the job does not
continue with incomplete outputs.
---
Nitpick comments:
In @.github/workflows/auto-update-matrix.yml:
- Around line 12-15: Add a workflow-level concurrency block to prevent
overlapping runs: at the top-level of the YAML (above jobs) add a concurrency
key with a stable group string like "auto-update-matrix-${{ github.ref }}" (or
include github.workflow/github.event_name as needed) and set cancel-in-progress:
true; this will ensure the detect-and-update job and any manual/scheduled runs
do not race on push/dispatch.
In `@source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page`:
- Around line 344-348: The option labels currently interpolate version variables
($latest_v, $latest_prb_v, $latest_nfb_v, $latest_beta_v, $latest_nos_v)
directly into the mk_option calls; update those label strings to escape the
version values using htmlspecialchars (e.g., htmlspecialchars($latest_v,
ENT_QUOTES, 'UTF-8')) before concatenation so mk_option receives HTML-escaped
labels for defense-in-depth while leaving the variable sources unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d82674e2-fbfb-422f-b14f-9afe88458511
📒 Files selected for processing (3)
.github/workflows/auto-update-matrix.yml.gitignoresource/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page
✅ Files skipped from review due to trivial changes (1)
- .gitignore
| # Fallback to NVIDIA developer forum if fetch or parse failed | ||
| if [ -z "${RAW_DATA}" ] || { [ -z "${PRB}" ] && [ -z "${NFB}" ] && [ -z "${BETA}" ]; }; then | ||
| RAW_DATA="$(wget -qO- https://forums.developer.nvidia.com/t/current-graphics-driver-releases/28500 | tidy -quiet -wrap 4096 2>/dev/null || true)" | ||
| PRB="$(echo "${RAW_DATA}" | grep -i "^Current production branch" | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')" | ||
| NFB="$(echo "${RAW_DATA}" | grep -i "^Current new feature branch" | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')" | ||
| BETA="$(echo "${RAW_DATA}" | grep -i "^Current beta" | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')" | ||
| LEGACY="$(echo "${RAW_DATA}" | grep -i -A1 '>Legacy releases' | tail -1 | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')" | ||
| fi | ||
|
|
||
| if [ -z "${RAW_DATA}" ]; then | ||
| echo "::error::Failed to fetch NVIDIA driver data from both sources" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Fail fast when required driver versions are still empty after fallback.
At Line 42, the guard only checks RAW_DATA. If HTML fetch succeeds but parsing misses key fields, the workflow continues with empty outputs and silently skips updates.
🛠️ Proposed fix
- if [ -z "${RAW_DATA}" ] || { [ -z "${PRB}" ] && [ -z "${NFB}" ] && [ -z "${BETA}" ]; }; then
+ if [ -z "${RAW_DATA}" ] || { [ -z "${PRB}" ] && [ -z "${NFB}" ]; }; then
RAW_DATA="$(wget -qO- https://forums.developer.nvidia.com/t/current-graphics-driver-releases/28500 | tidy -quiet -wrap 4096 2>/dev/null || true)"
PRB="$(echo "${RAW_DATA}" | grep -i "^Current production branch" | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')"
NFB="$(echo "${RAW_DATA}" | grep -i "^Current new feature branch" | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')"
BETA="$(echo "${RAW_DATA}" | grep -i "^Current beta" | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')"
LEGACY="$(echo "${RAW_DATA}" | grep -i -A1 '>Legacy releases' | tail -1 | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')"
fi
- if [ -z "${RAW_DATA}" ]; then
+ if [ -z "${RAW_DATA}" ] || { [ -z "${PRB}" ] && [ -z "${NFB}" ]; }; then
echo "::error::Failed to fetch NVIDIA driver data from both sources"
exit 1
fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/auto-update-matrix.yml around lines 33 - 45, The workflow
currently only fails when RAW_DATA is empty; update the fallback parsing block
to verify that required parsed variables (PRB, NFB, BETA, and LEGACY) are
present after fetching and parsing and fail fast if any are empty: after the
lines that set PRB, NFB, BETA, and LEGACY, add a guard that echoes an error
(e.g., "::error::Missing NVIDIA driver versions: ...") and exits non-zero when
any of PRB/NFB/BETA/LEGACY are empty so the job does not continue with
incomplete outputs.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/auto-update-matrix.yml (1)
37-49:⚠️ Potential issue | 🟠 MajorStill succeeds on partial NVIDIA parse results.
The fallback and final guard still treat
BETAandLEGACYas optional. If NVIDIA changes only those sections, this job exits successfully and silently leaves the beta/legacy tracks stale.🛠️ Suggested fix
- if [ -z "${RAW_DATA}" ] || { [ -z "${PRB}" ] && [ -z "${NFB}" ] && [ -z "${BETA}" ]; }; then + if [ -z "${RAW_DATA}" ] || [ -z "${PRB}" ] || [ -z "${NFB}" ] || [ -z "${BETA}" ] || [ -z "${LEGACY}" ]; then RAW_DATA="$(wget -qO- https://forums.developer.nvidia.com/t/current-graphics-driver-releases/28500 | tidy -quiet -wrap 4096 2>/dev/null || true)" PRB="$(echo "${RAW_DATA}" | grep -i "^Current production branch" | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')" NFB="$(echo "${RAW_DATA}" | grep -i "^Current new feature branch" | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')" BETA="$(echo "${RAW_DATA}" | grep -i "^Current beta" | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')" LEGACY="$(echo "${RAW_DATA}" | grep -i -A1 '>Legacy releases' | tail -1 | grep -oE '\b[0-9]+\.[0-9]+(\.[0-9]+)?\b')" fi - if [ -z "${RAW_DATA}" ] || { [ -z "${PRB}" ] && [ -z "${NFB}" ]; }; then - echo "::error::Failed to fetch NVIDIA driver data from both sources" + if [ -z "${RAW_DATA}" ] || [ -z "${PRB}" ] || [ -z "${NFB}" ] || [ -z "${BETA}" ] || [ -z "${LEGACY}" ]; then + echo "::error::Missing NVIDIA driver versions: PRB=${PRB:-<empty>} NFB=${NFB:-<empty>} BETA=${BETA:-<empty>} LEGACY=${LEGACY:-<empty>}" exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/auto-update-matrix.yml around lines 37 - 49, The fallback logic and final guard treat BETA and LEGACY as optional, allowing partial parses to pass; update the checks so that the wget/tidy fallback block and the final validation require PRB, NFB, BETA and LEGACY to be present (i.e., change occurrences of "{ [ -z \"${PRB}\" ] && [ -z \"${NFB}\" ]; }" and "{ [ -z \"${PRB}\" ] && [ -z \"${NFB}\" ]; }" to ensure all four variables PRB, NFB, BETA, and LEGACY are non-empty), and emit the error/exit if any of PRB, NFB, BETA or LEGACY are missing after both source attempts; keep using RAW_DATA for the fetch fallback as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/auto-update-matrix.yml:
- Around line 222-223: The current jq runtime tag check uses test("^[0-9]")
which only validates the first character; update the conditional tests for
$toolkit and $libnvidia (the expressions that set
.runtimes.containertoolkit.current and .runtimes.libnvidia.current) to use the
stricter regex test("^[0-9]+(\\.[0-9]+)+$") so only semantic versions like X.Y.Z
(or X.Y) pass and unstable tags (e.g. 1.19.0-rc1 or 1bad) are rejected.
- Around line 186-189: The compound command using "jq ... >
build-matrix.json.tmp && mv build-matrix.json.tmp build-matrix.json" masks jq
failures; change this to run the jq command and check its exit before moving the
tmp file so a jq error fails the step: run the jq redirection as a standalone
command (the "jq ... > build-matrix.json.tmp" invocation), ensure the shell
exits on non-zero (rely on set -e or explicitly test $? or use "|| exit 1"), and
only run "mv build-matrix.json.tmp build-matrix.json" if the jq command
succeeded, leaving the following "echo" and "cat build-matrix.json" unchanged.
---
Duplicate comments:
In @.github/workflows/auto-update-matrix.yml:
- Around line 37-49: The fallback logic and final guard treat BETA and LEGACY as
optional, allowing partial parses to pass; update the checks so that the
wget/tidy fallback block and the final validation require PRB, NFB, BETA and
LEGACY to be present (i.e., change occurrences of "{ [ -z \"${PRB}\" ] && [ -z
\"${NFB}\" ]; }" and "{ [ -z \"${PRB}\" ] && [ -z \"${NFB}\" ]; }" to ensure all
four variables PRB, NFB, BETA, and LEGACY are non-empty), and emit the
error/exit if any of PRB, NFB, BETA or LEGACY are missing after both source
attempts; keep using RAW_DATA for the fetch fallback as before.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de1295c9-bf7e-432e-9f05-8e0604f39ddd
📒 Files selected for processing (1)
.github/workflows/auto-update-matrix.yml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/auto-update-matrix.yml (1)
106-119: Minor robustness: GCC tag fallback could validate the retrieved value.If
build-matrix.jsonever lacks.gcc_tag,jq -rreturns the literal string"null", which would pass the!= ""check at line 143 and corrupt the field. This is unlikely given the current JSON structure, but for defense-in-depth:♻️ Proposed improvement
if [ -z "${GCC_TAG}" ]; then echo "::warning::Could not detect GCC tag, keeping current" - GCC_TAG=$(jq -r '.gcc_tag' build-matrix.json) + GCC_TAG=$(jq -r '.gcc_tag // ""' build-matrix.json) + if [ -z "${GCC_TAG}" ] || [ "${GCC_TAG}" = "null" ]; then + echo "::error::No valid gcc_tag found in build-matrix.json" + exit 1 + fi fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/auto-update-matrix.yml around lines 106 - 119, The current GCC tag detection writes whatever jq returns (including the literal "null") into GCC_TAG and exports it; update the logic around GCC_TAG so after attempting to detect it from the registry and before falling back to build-matrix.json you also validate that the value is non-empty and not the literal "null" (i.e., test for -z OR equal to "null"), and if invalid read fallback with jq -r '.gcc_tag' and validate that result as well; only write a valid tag to GITHUB_OUTPUT (and emit the warning/keep-current path if both are invalid) — check and update the shell variables GCC_TAG, the jq call that reads '.gcc_tag', and the final echo to "$GITHUB_OUTPUT" accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/auto-update-matrix.yml:
- Around line 106-119: The current GCC tag detection writes whatever jq returns
(including the literal "null") into GCC_TAG and exports it; update the logic
around GCC_TAG so after attempting to detect it from the registry and before
falling back to build-matrix.json you also validate that the value is non-empty
and not the literal "null" (i.e., test for -z OR equal to "null"), and if
invalid read fallback with jq -r '.gcc_tag' and validate that result as well;
only write a valid tag to GITHUB_OUTPUT (and emit the warning/keep-current path
if both are invalid) — check and update the shell variables GCC_TAG, the jq call
that reads '.gcc_tag', and the final echo to "$GITHUB_OUTPUT" accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a46221b-010b-4746-b37c-ba6128cb05e9
📒 Files selected for processing (1)
.github/workflows/auto-update-matrix.yml
|
I have had a look at the changes. With the changes in #8 it disables drivers that are not compatible with the card installed. But this PR shows all and does not disabled incompatible ones. 
There is no indication the driver is not working correctly. 
The PR does not seem to get the supported drivers for the GPU. The values were from my orginal plugin before coping the updated files. For the version what does pin mean i.e. is will not update. |
|
good catch. let me check. |
3 participants
Background
This PR grew organically from a concrete need: I have a Blackwell GPU (RTX Pro 6000) that requires the open-source kernel modules, and needed the 595 beta driver to get it running on Unraid. While redesigning the UI settings page to support open-source module selection and beta branches, we noticed security vulnerabilities in the existing shell scripts (command injection via unsanitized input, unsafe
rm -rfwith unquoted variables). Since we don't have a Jenkins instance, we also had to replace the build pipeline — the upside is that it now runs entirely on GitHub Actions with no external infrastructure needed, driven by a simple JSON config file.Rebased on current master to incorporate upstream changes including PR #8 (GPU support detection). Our card-based UI now integrates
card-support.phpfor architecture detection.Summary
card-support.php.auto-update-matrix.yml: Daily auto-detection of new NVIDIA driver versions and Unraid kernels, updates build matrix and versions.json. Old branch versions rotate toextra_buildsfor pin support. Also auto-detects GCC tag.build-nvidia.yml: JSON-driven matrix build with skip-check — only builds packages that don't already exist in the release.update_version()andchange_update_check()to prevent command injection via sed; function whitelist replacing unrestricted$@dispatcher; safe file cleanup preventing accidental mass deletion when variables are emptylatest_betaoption to download.sh, update-check.sh, and exec.shFETCH_VERSIONS()to eliminate redundant GitHub API call, fixed$DRIVERSvariable scope bugChanges by file
nvidia-driver.pageexec.shdownload.shupdate-check.shauto-update-matrix.ymlbuild-nvidia.ymlbuild-matrix.jsonTest plan
update_versionrejects invalid input (e.g.; rm -rf /)$LAT_PACKAGEis emptySummary by CodeRabbit
New Features
Bug Fixes
Chores