fix: OEM plugin issues #1288
fix: OEM plugin issues #1288
Conversation
WalkthroughThis pull request makes modifications across several areas. In the Unraid API authentication module, an obsolete import is removed and the directory path is updated. The rsync activation script simplifies remote user handling, removes legacy state file logic, and introduces a new flag to delete remote password files before synchronization. Additionally, legacy code for welcome modal injection in the activation code setup script is eliminated, with a comment noting an upcoming relocation of that functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as rsync Script
participant R as Remote Server
U->>S: Execute script with --remote-host & --remove-password flag
S->>R: Connect as root to remove /boot/config/passwd
R-->>S: Return deletion status (success or error)
S->>R: Execute rsync with flags --no-owner, --no-group, --no-perms
R-->>S: Send synchronization result
S->>U: Output success/error message
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (7)
🪧 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 (
|
elibosley
changed the title
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
plugin/scripts/rsync-activation-dir.sh(3 hunks)plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/data/server-state.php(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 (5)
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/data/server-state.php (1)
5-7: Added required dependenciesThe inclusion of Wrappers.php and Helpers.php appears to add necessary functionality. The use of
extract()withparse_plugin_cfg()imports configuration variables directly into the symbol table.plugin/scripts/rsync-activation-dir.sh (4)
5-8: Updated usage documentationThe usage instructions have been updated to remove the
--remote-userparameter and add the new--remove-passwordoption. The example usage provides a clear reference for users.
12-12: Added password removal optionThe new
--remove-passwordflag is properly implemented with a default value of false and appropriate option handling.Also applies to: 29-32
67-74: Password removal functionalityThe implementation to remove password files when the
--remove-passwordflag is set looks good. It handles errors appropriately and provides feedback to the user.However, consider adding a warning about the potential security implications of removing these files.
77-77: Modified rsync permissions handlingThe rsync command has been updated to include
--no-owner --no-group --no-permsflags, which prevents permission and ownership changes on the destination.This is a good change as it allows the destination system to maintain its own file permissions and ownership, preventing potential access issues.
.../source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/data/server-state.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugin/scripts/rsync-activation-dir.sh (1)
61-65:⚠️ Potential issueDestructive Remote Directory Cleaning
The script unconditionally removes all files in the specified remote directory using
rm -rf $REMOTE_PATH/*without any user confirmation. This operation is potentially destructive and could lead to unexpected data loss if users are not aware of its impact.Consider adding a confirmation prompt before executing this removal command to safeguard against accidental data loss.
🧹 Nitpick comments (1)
plugin/scripts/rsync-activation-dir.sh (1)
67-82: Conditional Removal of License Keys and Password FilesIn this block, the script prompts the user before removing Unraid license keys, which is good. However, the removal of the password file (and related sensitive files) is executed without additional confirmation. If these removals could have a significant impact, it might be worth adding a confirmation prompt for the password file deletion as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
plugin/scripts/rsync-activation-dir.sh(3 hunks)plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/data/server-state.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/data/server-state.php
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test API
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (5)
plugin/scripts/rsync-activation-dir.sh (5)
5-9: Usage Instructions UpdatedThe usage message and example have been updated to drop the obsolete
--remote-useroption and clearly require the--remote-hostparameter. This improves clarity for users. Consider verifying if the script name used in the example matches the actual filename for consistency.
12-12: Explicit Initialization of REMOVE_PASSWORDThe default value for
REMOVE_PASSWORDis now clearly initialized tofalse, ensuring that password removal is only triggered when explicitly requested.
29-32: Parsing the--remove-passwordFlagThe new command-line flag
--remove-passwordis correctly parsed and sets theREMOVE_PASSWORDvariable. This change aligns with the updated functionality.
41-50: Parameter Validation for Required ArgumentsThe validations for required parameters (
--local-directoryand--remote-host) are implemented well with clear error messages. This ensures the script fails early when necessary parameters are missing.
85-85: Enhanced Rsync Command FlagsIncluding the
--no-owner,--no-group, and--no-permsflags within the rsync command is a sensible choice for maintaining file permission consistency during synchronization. The command is also wrapped with error handling, which is a solid addition.
zackspear
force-pushed
the
fix/gridstack-activation-code-setup
branch
from
63a098e to
4201a9e
Compare
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.5.0](v4.4.1...v4.5.0) (2025-04-02) ### Features * add webgui theme switcher component ([#1304](#1304)) ([e2d00dc](e2d00dc)) * api plugin system & offline versioned dependency vendoring ([#1252](#1252)) ([9f492bf](9f492bf)) * **api:** add `unraid-api --delete` command ([#1289](#1289)) ([2f09445](2f09445)) * basic array controls ([#1291](#1291)) ([61fe696](61fe696)) * basic docker controls ([#1292](#1292)) ([12eddf8](12eddf8)) * copy to webgui repo script docs + wc build options ([#1285](#1285)) ([e54f189](e54f189)) ### Bug Fixes * additional url fixes ([4b2763c](4b2763c)) * **api:** redirect benign pnpm postinstall warning to log file ([#1290](#1290)) ([7fb7849](7fb7849)) * **deps:** update dependency chalk to v5 ([#1296](#1296)) ([6bed638](6bed638)) * **deps:** update dependency diff to v7 ([#1297](#1297)) ([3c6683c](3c6683c)) * disable all config watchers ([#1306](#1306)) ([5c1b435](5c1b435)) * extract callbacks to library ([#1280](#1280)) ([2266139](2266139)) * OEM plugin issues ([#1288](#1288)) ([d5a3d0d](d5a3d0d)) * replace files lost during pruning ([d0d2ff6](d0d2ff6)) --- 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>
3 participants
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
.set-password.phpfile restoration.