Skip to content

v10.1.1/ci fix #138

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.

Sign up for GitHub

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

Merged
gimlichael merged 1 commit into from
Dec 12, 2025
Merged

v10.1.1/ci fix #138

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 88 additions & 7 deletions .github/workflows/ci-pipeline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ jobs:
projects: ${{ matrix.project }}
build: true # we need to build due to xUnitv3
restore: true # we need to restore since we disabled caching
verbosity-level: diagnostic

test_windows:
name: call-test-windows
Expand All @@ -96,6 +97,7 @@ jobs:
test-arguments: -- RunConfiguration.DisableAppDomain=true
build: true # we need to build for .net48
restore: true # apparently we need to restore for .net48
verbosity-level: diagnostic

integration_test:
name: ⚗️ Integration Test
Expand Down Expand Up @@ -133,13 +135,92 @@ jobs:

- name: Fix Linux test apphost permissions
run: |
echo "Ensuring xUnit v3 test apphosts are executable on Linux..."
echo "Configuration: ${{ matrix.configuration }}"
# Dump a bit of info for debugging
find . -path "*/bin/${{ matrix.configuration }}/net*/*" -maxdepth 1 -type f -name "*Tests*" -print || true
# Actually fix permissions
find . -path "*/bin/${{ matrix.configuration }}/net*/*" -type f -name "*Tests*" -exec chmod +x {} +
echo "Permissions fixed."
set -euo pipefail

echo "=== Context ==="
echo "Runner: $RUNNER_OS / $RUNNER_ARCH"
echo "Configuration: ${{ inputs.configuration }}"
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The script references inputs.configuration but this job uses matrix.configuration instead. The integration_test job doesn't have access to inputs.configuration (which is only available in workflow_dispatch events), so this will evaluate to an empty string, causing the script to search for paths like */bin//net*/* which won't match the intended directories. This should be changed to matrix.configuration to match the job's configuration matrix.

Suggested change
echo "Configuration: ${{ inputs.configuration }}"
echo "Configuration: ${{ matrix.configuration }}"

Copilot uses AI. Check for mistakes.
echo "Workspace: $GITHUB_WORKSPACE"
echo "PWD: $(pwd)"
echo "Event: $GITHUB_EVENT_NAME"
echo "Ref: $GITHUB_REF"
echo "SHA: $GITHUB_SHA"
echo

echo "=== .NET info ==="
dotnet --info || true
echo

echo "=== Git state ==="
git rev-parse HEAD || true
git status --porcelain || true
git rev-parse --is-shallow-repository || true
echo

# Paths we care about
BIN_GLOB="*/bin/${{ inputs.configuration }}/net*/*"
OBJ_GLOB="*/obj/${{ inputs.configuration }}/net*/*"
Comment on lines +161 to +162
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The BIN_GLOB and OBJ_GLOB variables reference inputs.configuration which is not available in this job context. The integration_test job uses matrix.configuration instead. These paths will resolve incorrectly (e.g., */bin//net*/*), causing the find commands and chmod operations throughout the script to fail to locate the actual test binaries. Change both occurrences to matrix.configuration.

Suggested change
BIN_GLOB="*/bin/${{ inputs.configuration }}/net*/*"
OBJ_GLOB="*/obj/${{ inputs.configuration }}/net*/*"
BIN_GLOB="*/bin/${{ matrix.configuration }}/net*/*"
OBJ_GLOB="*/obj/${{ matrix.configuration }}/net*/*"

Copilot uses AI. Check for mistakes.

echo "=== Brute-force chmod (bin + obj) ==="
find . -type f \( -path "$BIN_GLOB" -o -path "$OBJ_GLOB" \) -exec chmod a+x {} + 2>/dev/null || true
Comment on lines +164 to +165
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Applying executable permissions to ALL files in bin and obj directories (chmod a+x on every file) is overly broad and could pose a security risk. Many files in these directories (like .dll, .pdb, .json, .xml, configuration files) should never be executable. This could inadvertently make data files or configuration files executable. Consider being more selective by only applying executable permissions to specific patterns that are actually test host executables (e.g., files named testhost, testhost.exe, files matching the test project names without extensions, or using file type detection first).

Suggested change
echo "=== Brute-force chmod (bin + obj) ==="
find . -type f \( -path "$BIN_GLOB" -o -path "$OBJ_GLOB" \) -exec chmod a+x {} + 2>/dev/null || true
echo "=== Selective chmod (testhost executables in bin + obj) ==="
find . \( -path "$BIN_GLOB" -o -path "$OBJ_GLOB" \) -type f \( -name "testhost" -o -name "testhost.exe" \) -exec chmod a+x {} + 2>/dev/null || true

Copilot uses AI. Check for mistakes.
echo "chmod completed (errors ignored)."
echo

echo "=== Mount options (look for noexec) ==="
# If binaries live on a noexec mount, chmod won't help.
mount | sed -n '1,200p' || true
echo

echo "=== Candidate executables (top 200) ==="
# Show what we might execute; exclude obvious managed files
find . -type f -path "$BIN_GLOB" \
! -name "*.dll" ! -name "*.pdb" ! -name "*.json" ! -name "*.xml" \
-printf "%m %u:%g %s %p\n" | head -n 200 || true
Comment on lines +176 to +178
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The find command uses -printf which is a GNU findutils extension. While Ubuntu runners should support this, the use of GNU-specific options could cause portability issues if the script is ever adapted for other platforms. The same applies to sed -n '1,200p' which could be replaced with head -n 200. Consider documenting this GNU dependency or using more portable alternatives like find ... -ls or find ... | xargs ls -la for broader compatibility.

Copilot uses AI. Check for mistakes.
echo

echo "=== Likely xUnit / test hosts (if present) ==="
# These names vary; do not rely on just *Tests*
find . -type f -path "$BIN_GLOB" \( \
-name "testhost*" -o \
-name "*xunit*" -o \
-name "*Tests*" -o \
-name "*.runsettings" \
\) -printf "%m %u:%g %s %p\n" | head -n 200 || true
echo

echo "=== Deep diagnostics for any 'testhost' or apphost candidates ==="
# For each likely executable, show the facts that explain 'permission denied' vs 'exec format error'
while IFS= read -r f; do
echo "--- $f ---"
ls -la "$f" || true

# Identify file type and architecture
file -L "$f" || true

# If it's an ELF binary, show its dynamic interpreter and linked libs (exec format errors often show up here)
if file -L "$f" | grep -q "ELF"; then
echo "readelf -l (interpreter):"
readelf -l "$f" 2>/dev/null | sed -n '1,80p' || true
echo "ldd (dependencies):"
ldd "$f" 2>/dev/null || true
fi

# If it's a script, CRLF in the shebang can cause 'Exec format error'
if head -c 2 "$f" 2>/dev/null | grep -q "#!"; then
echo "shebang:"
head -n 1 "$f" | cat -A || true
fi

echo
done < <(
find . -type f -path "$BIN_GLOB" \( \
-name "testhost*" -o \
-name "*xunit*" -o \
-name "*Tests*" \
\) | head -n 50
) || true
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The process substitution syntax done < <(find ...) on line 215 is a bash-specific feature. While the script explicitly sets shell: bash on line 224, it's good that this is specified. However, the set -euo pipefail on line 138 combined with the || true on line 221 means that if the process substitution or the while loop fails, it will be silently ignored. Consider whether you want to catch failures in the diagnostic loop itself, or if silent failure is acceptable for this diagnostic-only step.

Suggested change
) || true
)

Copilot uses AI. Check for mistakes.
Comment on lines +191 to +221
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The diagnostic script processes up to 50 test binaries with extensive checks (file, readelf, ldd commands) which could significantly increase CI run time, especially with many test projects. While this is valuable for debugging, consider adding a condition to only run this extensive diagnostic when tests are actually failing, or make it configurable via an input parameter. This would reduce noise and CI time in successful runs while still providing diagnostics when needed.

Copilot uses AI. Check for mistakes.

echo "=== Done diagnostics step ==="
shell: bash
Comment on lines 136 to 224
Copy link

@coderabbitai coderabbitai bot Dec 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Let me verify the exact lines using the matrix and inputs context
sed -n '9,50p' .github/workflows/ci-pipeline.yml | cat -n

Repository: codebeltnet/cuemon

Length of output: 1418

🏁 Script executed:

# Verify the exact usage of configuration variable in the diagnostic step
sed -n '136,224p' .github/workflows/ci-pipeline.yml | grep -n "configuration\|ldd"

Repository: codebeltnet/cuemon

Length of output: 343

🏁 Script executed:

# Find the job definition that contains the diagnostic step
sed -n '100,300p' .github/workflows/ci-pipeline.yml | head -n 200

Repository: codebeltnet/cuemon

Length of output: 6397

🏁 Script executed:

# Search for the test job and its matrix
grep -n "test:" .github/workflows/ci-pipeline.yml

Repository: codebeltnet/cuemon

Length of output: 104

🏁 Script executed:

# Get the full structure around lines 100-250 to see job definitions
sed -n '80,250p' .github/workflows/ci-pipeline.yml | cat -n

Repository: codebeltnet/cuemon

Length of output: 7281

Use matrix.configuration instead of undefined inputs.configuration in the diagnostic step. The integration_test job defines and uses matrix.configuration throughout (download artifacts, test execution), but this diagnostic step incorrectly references ${{ inputs.configuration }}, which is undefined in this context, causing the chmod and find globs to fail.

-          echo "Configuration: ${{ inputs.configuration }}"
+          echo "Configuration: ${{ matrix.configuration }}"
...
-          BIN_GLOB="*/bin/${{ inputs.configuration }}/net*/*"
-          OBJ_GLOB="*/obj/${{ inputs.configuration }}/net*/*"
+          BIN_GLOB="*/bin/${{ matrix.configuration }}/net*/*"
+          OBJ_GLOB="*/obj/${{ matrix.configuration }}/net*/*"

Replace ldd with non-executing ELF inspection tools. The ldd command executes code from binaries to resolve dependencies, which is unnecessary in a CI environment and presents an avoidable security risk. Use readelf -d and objdump -p instead to inspect NEEDED sections without execution.

-              echo "ldd (dependencies):"
-              ldd "$f" 2>/dev/null || true
+              echo "readelf -d (NEEDED):"
+              readelf -d "$f" 2>/dev/null | sed -n '1,120p' || true
+              echo "objdump -p (NEEDED):"
+              objdump -p "$f" 2>/dev/null | sed -n '1,120p' || true
🤖 Prompt for AI Agents 
.github/workflows/ci-pipeline.yml around lines 136 to 224: the diagnostic step
incorrectly references the undefined inputs.configuration and calls ldd (which
executes code); update all occurrences of ${{ inputs.configuration }} to ${{
matrix.configuration }} (including BIN_GLOB and OBJ_GLOB) so the find/chmod
globs work, and remove the ldd "$f" call — replace it with non-executing
inspection commands such as readelf -d "$f" and objdump -p "$f" (run both
conditionally for ELF files) to show NEEDED/DT entries without executing the
binary.


- name: Test with ${{ matrix.configuration }} build
Expand Down
Loading