fix: if user script is executable, execute it

[wolf31o2]

Summary by cubic

Execute the user startup script directly, respecting its shebang, instead of forcing bash. If it isn’t executable, set +x and run it; log a warning if the script exits non-zero.

^{Written for commit 1170325. Summary will update automatically on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Improved startup script execution with automatic permission management when scripts aren't initially executable.
  • Enhanced error detection and reporting with warnings logged when startup scripts fail during execution.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The bin/entrypoint script is enhanced to improve USER_STARTUP_SCRIPT execution handling. A return-tracking variable is introduced to capture script exit codes. The implementation checks if the script is executable; if not, it automatically applies executable permissions before running. Upon execution, the exit code is captured, and if non-zero, a warning is emitted to stderr. No public exports or interfaces are modified.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Permission checking logic implementation and correctness
• Error warning mechanism to stderr
• Return code capture and propagation to ensure exit codes are properly tracked

Possibly related PRs

• fix(entrypoint): bash the user script #19: Modifies USER_STARTUP_SCRIPT execution in bin/entrypoint with related changes to script handling and execution semantics

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: enhancing USER_STARTUP_SCRIPT execution by checking if the script is executable and handling it appropriately.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/exec-if-executable

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[cubic-dev-ai]

No issues found across 1 file

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a11d539 and 1170325.

📒 Files selected for processing (1)

• bin/entrypoint (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: cubic · AI code reviewer

🔇 Additional comments (1)

bin/entrypoint (1)

38-40: Verify intended behavior when user script fails.

The entrypoint continues execution and starts OpenVPN even when the user startup script exits with a non-zero code. Is this the intended behavior, or should the entrypoint exit with an error when the user script fails?

If the entrypoint should fail when the user script fails, apply this diff:

 	if [[ ${__ret} -ne 0 ]]; then
 		echo "WARN: User startup script exited with error code ${__ret}" >&2
+		exit ${__ret}
 	fi

Otherwise, if the current behavior is intentional (allowing OpenVPN to start regardless), consider changing "WARN:" to "INFO:" to better reflect that this is an expected scenario where startup continues.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fix chmod failure handling and improve security.

The current implementation has several issues:

1. Logic error: If chmod fails on line 34, the || ls -l runs for debugging, but execution continues to line 35 where the non-executable script will fail. The fallback should either exit or skip execution.

2. Overly permissive: chmod a+x grants execute permission to all users. Use chmod +x instead.

3. Missing quotes: ${USER_STARTUP_SCRIPT} should be quoted to handle paths with spaces.

Apply this diff to fix the issues:

 	__ret=0
 	if [[ -x ${USER_STARTUP_SCRIPT} ]]; then
-		${USER_STARTUP_SCRIPT}
+		"${USER_STARTUP_SCRIPT}"
 		__ret=$?
 	else
-		chmod a+x ${USER_STARTUP_SCRIPT} || ls -l ${USER_STARTUP_SCRIPT}
-		${USER_STARTUP_SCRIPT}
-		__ret=$?
+		if chmod +x "${USER_STARTUP_SCRIPT}"; then
+			"${USER_STARTUP_SCRIPT}"
+			__ret=$?
+		else
+			echo "ERROR: Failed to make user startup script executable" >&2
+			ls -l "${USER_STARTUP_SCRIPT}" >&2
+			__ret=1
+		fi
 	fi

📝 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.

Suggested change

	if [[ -x ${USER_STARTUP_SCRIPT} ]]; then
	${USER_STARTUP_SCRIPT}
	__ret=$?
	else
	chmod a+x ${USER_STARTUP_SCRIPT} || ls -l ${USER_STARTUP_SCRIPT}
	${USER_STARTUP_SCRIPT}
	__ret=$?
	fi
	__ret=0
	if [[ -x ${USER_STARTUP_SCRIPT} ]]; then
	"${USER_STARTUP_SCRIPT}"
	__ret=$?
	else
	if chmod +x "${USER_STARTUP_SCRIPT}"; then
	"${USER_STARTUP_SCRIPT}"
	__ret=$?
	else
	echo "ERROR: Failed to make user startup script executable" >&2
	ls -l "${USER_STARTUP_SCRIPT}" >&2
	__ret=1
	fi
	fi

🤖 Prompt for AI Agents

In bin/entrypoint around lines 30 to 37, the block attempts to chmod and run the
USER_STARTUP_SCRIPT but has logic and security issues: replace chmod a+x with
chmod +x, quote "${USER_STARTUP_SCRIPT}" everywhere, and change the flow so that
after attempting chmod +x "${USER_STARTUP_SCRIPT}" you check its exit status and
if chmod fails run ls -l "${USER_STARTUP_SCRIPT}" and exit with a non-zero code
(or skip execution) instead of proceeding to run a non-executable file; finally
only execute the script if it is -x "${USER_STARTUP_SCRIPT}" and propagate its
exit code to __ret.