Skip to content

fix(tests): '\$[' is read as the deprecated $[...] arithmetic form #60

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

Open
hyperpolymath wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(tests): '\$[' is read as the deprecated $[...] arithmetic form #60

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
2 changes: 1 addition & 1 deletion tests/security-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
TESTS_PASSED=$((TESTS_PASSED + 1))
return 0
else
if [ "${severity}" = "warning" ]; then

Check failure on line 21 in tests/security-tests.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich. 


See more on https://sonarcloud.io/project/issues?id=hyperpolymath_qubes-sdp&issues=AaA_G4RfWyVTeQoeAJgD&open=AaA_G4RfWyVTeQoeAJgD&pullRequest=60
echo " ⚠ ${description}"
WARNINGS=$((WARNINGS + 1))
return 0
Expand All @@ -36,7 +36,7 @@
# Test configuration security
echo "Testing configuration security:"

if [ -f "../qubes-config.conf" ]; then

Check failure on line 39 in tests/security-tests.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich. 


See more on https://sonarcloud.io/project/issues?id=hyperpolymath_qubes-sdp&issues=AaA_G4RfWyVTeQoeAJgE&open=AaA_G4RfWyVTeQoeAJgE&pullRequest=60
source ../qubes-config.conf 2>/dev/null || true

# Check vault has no network
Expand All @@ -62,14 +62,14 @@

# Check for dangerous commands in scripts
for script in ../qubes-setup*.sh; do
if [ -f "${script}" ]; then

Check failure on line 65 in tests/security-tests.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich. 


See more on https://sonarcloud.io/project/issues?id=hyperpolymath_qubes-sdp&issues=AaA_G4RfWyVTeQoeAJgF&open=AaA_G4RfWyVTeQoeAJgF&pullRequest=60
script_name=$(basename "${script}")

# Should not have rm -rf / or similar
test_assert "${script_name} doesn't contain 'rm -rf /'" "! grep -q 'rm -rf /' \"${script}\""

# Should not have eval of user input without validation
test_assert "${script_name} doesn't have unsafe eval" "! grep -E 'eval .*\\$[{]?[a-zA-Z_]+' \"${script}\" | grep -qv '#'"
test_assert "${script_name} doesn't have unsafe eval" "! grep -E 'eval .*\\\$[{]?[a-zA-Z_]+' \"${script}\" | grep -qv '#'"

@codacy-production codacy-production Bot Aug 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM RISK

The 'eval' safety check has two significant issues: 1) It is easily bypassed by trailing comments (e.g., 'eval $VAR # comment') because the current implementation discards any line containing a hash. 2) The regex '[a-zA-Z_]+' fails to detect positional parameters ($1, $2) or special variables ($@, $*), which are common sources of injection. Consider replacing this check with a more robust regex that ignores leading comments but captures a broader set of variable patterns: ! grep -E '^[[:space:]]*[^#]*eval .*\\\$[({]?[a-zA-Z0-9_*@?]'.


# Should check for dom0
test_assert "${script_name} checks for dom0" "grep -q 'hostname.*dom0' \"${script}\""
Expand All @@ -83,7 +83,7 @@

# Scripts should be executable but not world-writable
for script in ../*.sh ../tools/*.sh; do
if [ -f "${script}" ]; then

Check failure on line 86 in tests/security-tests.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich. 


See more on https://sonarcloud.io/project/issues?id=hyperpolymath_qubes-sdp&issues=AaA_G4RfWyVTeQoeAJgG&open=AaA_G4RfWyVTeQoeAJgG&pullRequest=60
script_name=$(basename "${script}")

test_assert "${script_name} is executable" "[ -x \"${script}\" ]"
Expand All @@ -97,7 +97,7 @@
echo "Testing for hardcoded credentials:"

for file in ../*.sh ../*.conf; do
if [ -f "${file}" ]; then

Check failure on line 100 in tests/security-tests.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich. 


See more on https://sonarcloud.io/project/issues?id=hyperpolymath_qubes-sdp&issues=AaA_G4RfWyVTeQoeAJgH&open=AaA_G4RfWyVTeQoeAJgH&pullRequest=60
filename=$(basename "${file}")

# Check for passwords
Expand All @@ -113,7 +113,7 @@
# Test Salt Stack security
echo "Testing Salt Stack security:"

if [ -f "../qubes-salt/qubes-sdp.sls" ]; then

Check failure on line 116 in tests/security-tests.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich. 


See more on https://sonarcloud.io/project/issues?id=hyperpolymath_qubes-sdp&issues=AaA_G4RfWyVTeQoeAJgI&open=AaA_G4RfWyVTeQoeAJgI&pullRequest=60
# Vault should have no network
test_assert "Salt: vault has no network" "grep -A 5 '^vault:' ../qubes-salt/qubes-sdp.sls | grep -q 'netvm: \"\"'"

Expand All @@ -127,11 +127,11 @@
echo "Testing for unsafe practices:"

for script in ../*.sh ../tools/*.sh; do
if [ -f "${script}" ]; then

Check failure on line 130 in tests/security-tests.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich. 


See more on https://sonarcloud.io/project/issues?id=hyperpolymath_qubes-sdp&issues=AaA_G4RfWyVTeQoeAJgJ&open=AaA_G4RfWyVTeQoeAJgJ&pullRequest=60
script_name=$(basename "${script}")

# Should not use curl without verification
test_assert "${script_name} doesn't use curl without verification" "! grep 'curl.*-k' \"${script}\"" "warning"

Check warning on line 134 in tests/security-tests.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of using the literal 'warning' 7 times. 


See more on https://sonarcloud.io/project/issues?id=hyperpolymath_qubes-sdp&issues=AaA_G4RfWyVTeQoeAJgM&open=AaA_G4RfWyVTeQoeAJgM&pullRequest=60

# Should not download and execute directly
test_assert "${script_name} doesn't pipe curl to shell" "! grep -E 'curl.*\\|.*sh' \"${script}\""
Expand All @@ -146,10 +146,10 @@
echo " Failed: ${TESTS_FAILED}"
echo " Warnings: ${WARNINGS}"

if [ ${TESTS_FAILED} -eq 0 ]; then

Check failure on line 149 in tests/security-tests.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich. 


See more on https://sonarcloud.io/project/issues?id=hyperpolymath_qubes-sdp&issues=AaA_G4RfWyVTeQoeAJgK&open=AaA_G4RfWyVTeQoeAJgK&pullRequest=60
echo ""
echo "All critical security tests passed!"
if [ ${WARNINGS} -gt 0 ]; then

Check failure on line 152 in tests/security-tests.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich. 


See more on https://sonarcloud.io/project/issues?id=hyperpolymath_qubes-sdp&issues=AaA_G4RfWyVTeQoeAJgL&open=AaA_G4RfWyVTeQoeAJgL&pullRequest=60
echo "Review warnings above for potential improvements."
fi
exit 0
Expand Down
Loading