-
Couldn't load subscription status.
- Fork 11
feat: add ReplaceKey functionality to plugin #1264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Introduced a new ReplaceKey.php file for handling key validation and installation. - Updated dynamix.unraid.net.plg to include the new ReplaceKey.php in restore and preserve files. This change enhances the plugin's capability to manage license keys effectively.
…pdate, Registration - Changed the `check()` method call in Registration.page to use the `force` parameter. - Moved the `require_once` for reboot-details.php in Downgrade.page and Update.page to ensure it's included after the ReplaceKey check.
WalkthroughThis pull request introduces a new PHP class, Changes
Sequence Diagram(s)sequenceDiagram
participant S as Unraid Check Script
participant RK as ReplaceKey
participant UO as UnraidOsCheck
S->>RK: Instantiate ReplaceKey
S->>RK: call check() (or check(true) in Registration)
RK-->>S: Return key validation/installation result
S->>UO: Include UnraidCheck.php and instantiate UnraidOsCheck
UO-->>S: Proceed with update check
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (6)
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.plugin.manager/scripts/unraidcheck (1)
17-21: Consider error handling after calling$replaceKey->check().Currently, any internal error from
$replaceKey->check()is silently ignored. To improve resilience, you could log or handle possible failures (e.g., JSON error files) so that the update check can display or act upon license-related issues.require_once "$docroot/plugins/dynamix/include/ReplaceKey.php"; $replaceKey = new ReplaceKey(); -$replaceKey->check(); +$checkResult = $replaceKey->check(); +if ($checkResult === false) { + // For instance, log or notify about ReplaceKey check failure + // This helps surface errors from ReplaceKey +} require_once "$docroot/plugins/dynamix.plugin.manager/include/UnraidCheck.php";plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix/include/ReplaceKey.php (5)
1-3: Check for invalid docroot paths.The
$docrootfallback is correct, but consider verifying that$docrootpoints to a valid location to avoid file-inclusion issues in future expansions.
4-28: Validate constructor inputs.The constructor sets crucial properties (
guid,keyfile, etc.) silently using@. Consider validating the file read/parse_ini_fileresults for better error reporting and reliability.
72-109: Handle empty or malformed JSON gracefully.When
json_decode($response, true)fails, an empty array is returned. You might log a warning or store a diagnostic message if validation fails unexpectedly.
141-185: Mind potential concurrency ininstallNewKey().Installation modifies system state and writes to
/tmp/ReplaceKey/error.json. If multiple key replacements are triggered, concurrency might cause partial writes or race conditions. Consider adding locking or checks for concurrent calls.
187-195: Checkfile_put_contentsreturn value.
file_put_contentscan fail and returnfalse. If the filesystem is read-only or space is full, the error JSON won’t be written. Handling that return gracefully can help in diagnosing deeper issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (7)
plugin/plugins/dynamix.unraid.net.plg(2 hunks)plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Registration.page(1 hunks)plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.plugin.manager/Downgrade.page(1 hunks)plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.plugin.manager/Update.page(1 hunks)plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.plugin.manager/scripts/unraidcheck(1 hunks)plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix/include/ReplaceKey.php(1 hunks)web/composables/services/webgui.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (10)
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.plugin.manager/Downgrade.page (2)
19-22: Integration of ReplaceKey functionalityThe code correctly adds the ReplaceKey class instantiation and check method call. This implements the license key validation functionality before proceeding with downgrade operations.
23-23: Moved reboot-details.php inclusionThe require_once statement for reboot-details.php has been appropriately repositioned to execute after the ReplaceKey check. This ensures proper sequencing where license key validation occurs before reboot details are processed.
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Registration.page (1)
19-19: Added force parameter to check methodThe change adds a boolean parameter (true) to the ReplaceKey->check() method call, which implements the force check functionality mentioned in the PR objectives. This ensures the key validation is performed regardless of any cached state.
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.plugin.manager/Update.page (2)
16-18: Integration of ReplaceKey functionalityThe code correctly adds the ReplaceKey class instantiation and check method call, implementing license key validation before proceeding with update operations.
20-21: Moved reboot-details.php inclusion and RebootDetails instantiationThe require_once statement for reboot-details.php and the RebootDetails instantiation have been properly repositioned to execute after the ReplaceKey check. This ensures the correct sequence of operations where license key validation occurs before reboot details are processed.
web/composables/services/webgui.ts (1)
111-111: TODO comment for future URL updateA TODO comment has been added indicating that the URL should be replaced with '/scripts/unraidcheck' in the future. This is a good documentation practice that flags the need for a future code change without affecting current functionality.
plugin/plugins/dynamix.unraid.net.plg (2)
387-387: File restoration addition looks good.Adding
ReplaceKey.phptoFILES_TO_RESTOREensures the file is correctly restored after updates or uninstalls. No immediate concerns.
509-509: Correct approach to prevent file downgrade.Including
ReplaceKey.phpinpreserveFilesDirswithpreventDowngradeprevents older plugin packages from overwriting the new file, maintaining version integrity.plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix/include/ReplaceKey.php (2)
30-70: Store or handle cURL errors.A
@todoexists to store cURL errors, but it's not implemented. Logging these errors could be essential for debugging misconfigurations or network failures.Would you like a shell script to confirm that no other references handle the cURL errors, ensuring we don’t miss potential error-handling implementations?
111-139: Confirm license format ingetLatestKey().Currently, any non-empty
'license'field is assumed valid. Consider minimal checks to ensure that the returned string is indeed a valid key or a well-formed license file content.
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix/include/ReplaceKey.php
Show resolved
Hide resolved
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.4.0](v4.3.1...v4.4.0) (2025-03-25) ### Features * add ReplaceKey functionality to plugin ([#1264](#1264)) ([4aadcef](4aadcef)) * downgrade page replace key check ([#1263](#1263)) ([8d56d12](8d56d12)) * make log viewer component dynamic ([#1242](#1242)) ([e6ec110](e6ec110)) * ReplaceKey functionality in Registration and Update pages ([#1246](#1246)) ([04307c9](04307c9)) * UnraidCheckExec for Check OS Updates via UPC dropdown ([#1265](#1265)) ([5935a3b](5935a3b)) ### Bug Fixes * **deps:** update all non-major dependencies ([#1236](#1236)) ([7194f85](7194f85)) * **deps:** update all non-major dependencies ([#1247](#1247)) ([20b0aeb](20b0aeb)) * **deps:** update all non-major dependencies ([#1251](#1251)) ([33a1a1d](33a1a1d)) * **deps:** update all non-major dependencies ([#1253](#1253)) ([53fec0e](53fec0e)) * **deps:** update dependency @nestjs/passport to v11 ([#1244](#1244)) ([edc93a9](edc93a9)) * **deps:** update dependency graphql-subscriptions to v3 ([#1209](#1209)) ([c14c85f](c14c85f)) * **deps:** update dependency ini to v5 ([#1217](#1217)) ([f27660f](f27660f)) * **deps:** update dependency jose to v6 ([#1248](#1248)) ([42e3d59](42e3d59)) * **deps:** update dependency marked to v15 ([#1249](#1249)) ([2b6693f](2b6693f)) * **deps:** update dependency pino-pretty to v13 ([#1250](#1250)) ([85fb910](85fb910)) * **deps:** update dependency pm2 to v6 ([#1258](#1258)) ([04ad2bc](04ad2bc)) * **deps:** update dependency shadcn-vue to v1 ([#1259](#1259)) ([1a4fe8f](1a4fe8f)) * **deps:** update dependency vue-i18n to v11 ([#1261](#1261)) ([0063286](0063286)) * **deps:** update vueuse monorepo to v13 (major) ([#1262](#1262)) ([94caae3](94caae3)) * make scripts executable when building the plugin ([#1255](#1255)) ([e237f38](e237f38)) * node installation not persisting across reboots ([#1256](#1256)) ([0415cf1](0415cf1)) * update configValid state to ineligible in var.ini and adjust rel… ([#1268](#1268)) ([ef8c954](ef8c954)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This change enhances the plugin's capability to manage license keys effectively.
ReplaceKey.phpfrom the webgui repo for handling auto-extended key check & installationcheck()method call inRegistration.pageto use theforceparameter per https://app.asana.com/0/1204220153625175/1209573221367693/frequire_onceforreboot-details.phpin Downgrade.page and Update.page to ensure it's included after theReplaceKeycheck.Summary by CodeRabbit
New Features
Chores