Skip to content

7.3 Adjust logging of inline FILE execution#2422

Merged
limetech merged 1 commit into
unraid:masterfrom
Squidly271:7.3-pluginlogging
Oct 15, 2025
Merged

7.3 Adjust logging of inline FILE execution#2422
limetech merged 1 commit into
unraid:masterfrom
Squidly271:7.3-pluginlogging

Conversation

@Squidly271

@Squidly271 Squidly271 commented Oct 14, 2025

Copy link
Copy Markdown
Contributor

For 7.3.

Adjust logging during plugin installation / update / removal to indicate the plugin and FILE section being executed.

Support edge case of a plugin installing another plugin via inline FILE

Summary by CodeRabbit

  • Bug Fixes
    • Prevents conflicts when processing multiple FILE elements by generating a unique temporary script for each inline section.
    • Ensures safer execution of inline scripts and consistent cleanup after running.
  • Chores
    • Adds clearer logging around inline script execution to aid troubleshooting and visibility.

@Squidly271 Squidly271 changed the title Adjust logging of inline FILE execution 7.3 Adjust logging of inline FILE execution Oct 14, 2025
@coderabbitai

coderabbitai Bot commented Oct 14, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Implements per-FILE inline script handling by assigning a unique temp filename per file, writing INLINE content to it, logging execution, running it via an escaped command string, and removing the temp file afterward. Replaces the prior static /tmp/inline.sh usage and generic logging.

Changes

Cohort / File(s) Summary of changes
Inline script execution handling
emhttp/plugins/dynamix.plugin.manager/scripts/plugin
- Add per-file counter current_file and increment per FILE element
- Generate unique temp path for INLINE scripts using counter and plugin filename
- Write INLINE content to that path
- Execute via composed command with escapeshellarg
- Add explicit execution log message
- Remove static /tmp/inline.sh usage and generic log line
- Preserve temp file cleanup

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant PluginManager as Plugin Manager
  participant Parser as FILE/INLINE Parser
  participant FS as Temp FS
  participant Shell as Shell

  User->>PluginManager: Install/Process plugin
  PluginManager->>Parser: Iterate FILE elements
  loop For each FILE
    Parser->>PluginManager: INLINE content detected
    note over PluginManager: Increment current_file
    PluginManager->>FS: Create temp script path (per-file)
    PluginManager->>FS: Write INLINE content
    PluginManager->>PluginManager: Build escaped command
    PluginManager->>Shell: Execute temp script
    Shell-->>PluginManager: Exit status
    PluginManager->>FS: Remove temp script
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through files, count one by one,
Craft temp scripts, then I’m done.
Log a squeak, escape the string,
Run and clean—what tidy spring!
Carrot-colored prompts, no crumbs to see—
Each INLINE nibble, uniquely free. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed Title succinctly captures the primary change of enhancing logging for inline FILE execution in the 7.3 release, directly reflecting the modifications described in the changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

🔧 PR Test Plugin Available

A test plugin has been generated for this PR that includes the modified files.

Version: 2025.10.14.1728
Build: View Workflow Run

📥 Installation Instructions:

Install via Unraid Web UI:

  1. Go to Plugins → Install Plugin
  2. Copy and paste this URL:
https://preview.dl.unraid.net/pr-plugins/pr-2422/webgui-pr-2422.plg
  1. Click Install

Alternative: Direct Download

⚠️ Important Notes:

  • Testing only: This plugin is for testing PR changes
  • Backup included: Original files are automatically backed up
  • Easy removal: Files are restored when plugin is removed
  • Conflicts: Remove this plugin before installing production updates

📝 Modified Files:

Click to expand file list
emhttp/plugins/dynamix.plugin.manager/scripts/plugin

🔄 To Remove:

Navigate to Plugins → Installed Plugins and remove webgui-pr-2422, or run:

plugin remove webgui-pr-2422

🤖 This comment is automatically generated and will be updated with each new push to this PR.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
emhttp/plugins/dynamix.plugin.manager/scripts/plugin (1)

466-471: Excellent improvements to security, logging, and collision prevention.

The changes successfully address the PR objectives:

  • Unique temp filenames prevent collisions when multiple inline scripts execute (including the edge case of nested plugin installations)
  • Enhanced logging clearly indicates which plugin and FILE section is executing
  • escapeshellarg() properly prevents command injection vulnerabilities

Consider applying the same content handling used for named INLINE files for consistency:

-        file_put_contents($name, $file->INLINE);
+        $contents = trim($file->INLINE).PHP_EOL;
+        file_put_contents($name, $contents);

This ensures shell scripts have a trailing newline, following the same pattern used at lines 404-405.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 95bc896 and 3c71758.

📒 Files selected for processing (1)
  • emhttp/plugins/dynamix.plugin.manager/scripts/plugin (2 hunks)
🔇 Additional comments (1)
emhttp/plugins/dynamix.plugin.manager/scripts/plugin (1)

345-347: LGTM! Counter implementation is correct.

The per-file counter initialization and increment logic is properly placed and will provide unique identifiers for each FILE element, starting from 1.

@ljm42 ljm42 added the 7.3 label Oct 14, 2025
@limetech limetech merged commit f22fd5b into unraid:master Oct 15, 2025
2 checks passed
@Squidly271 Squidly271 deleted the 7.3-pluginlogging branch November 5, 2025 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants