Skip to content

Add rbash test#104

Merged
sachintu47 merged 2 commits intozopencommunity:mainfrom
sabi789:main
Apr 28, 2026
Merged

Add rbash test#104
sachintu47 merged 2 commits intozopencommunity:mainfrom
sabi789:main

Conversation

@sabi789
Copy link
Copy Markdown
Member

@sabi789 sabi789 commented Apr 27, 2026

Add rbash Test

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Apr 27, 2026

🤖 Augment PR Summary

Summary: Adds an rbash-focused regression test to the bashport stable patch set.

Changes:

  • Updates the upstream MANIFEST (via stable-patches/MANIFEST.patch) to include new rbash test artifacts.
  • Adds a new test script tests/rbash.tests that exercises common restricted-shell prohibitions (cd, PATH changes, redirection, exec, etc.).
  • Adds tests/rbash.right as the expected output for the new test.
  • Adds tests/run-rbash to integrate the new test into the standard run-* harness and produce rbash.output on failure.

Technical Notes: The new test primarily validates restrictions by checking non-zero exit codes from rbash -c invocations and diffs the captured output against rbash.right.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

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

Review completed. 5 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread stable-patches/tests/rbash_test.patch Outdated
+ echo "Testing: $test_name"
+
+ # Run command in rbash and capture exit code
+ rbash -c "$command" 2>/dev/null
Copy link
Copy Markdown

@augmentcode augmentcode Bot Apr 27, 2026

Choose a reason for hiding this comment

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

stable-patches/tests/rbash_test.patch:29: rbash is invoked by name, which may resolve to a system-installed rbash (or be missing) during make check, making the test non-hermetic. Consider invoking the just-built shell in restricted mode (e.g., via ${THIS_SH}) so the test validates the ported binary.

Severity: medium

Other Locations
  • stable-patches/tests/rbash_test.patch:69

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Comment thread stable-patches/tests/rbash_test.patch Outdated
+}
+
+# Test 1: Cannot change directory
+test_restriction "cd restriction" "cd /tmp"
Copy link
Copy Markdown

@augmentcode augmentcode Bot Apr 27, 2026

Choose a reason for hiding this comment

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

stable-patches/tests/rbash_test.patch:44: Using hardcoded paths like /tmp and /bin/ls can make the restriction checks pass/fail for environmental reasons (missing directory/binary, permissions) rather than because the restriction is enforced. That can reduce the test’s ability to catch regressions.

Severity: medium

Other Locations
  • stable-patches/tests/rbash_test.patch:50
  • stable-patches/tests/rbash_test.patch:53
  • stable-patches/tests/rbash_test.patch:62
  • stable-patches/tests/rbash_test.patch:65

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Comment thread stable-patches/tests/rbash_test.patch Outdated
+ ✓ PASS: Command properly restricted (exit code: 1)
+
+Testing: command with /
+ ✓ PASS: Command properly restricted (exit code: 127)
Copy link
Copy Markdown

@augmentcode augmentcode Bot Apr 27, 2026

Choose a reason for hiding this comment

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

stable-patches/tests/rbash_test.patch:108: rbash.right expects a specific exit code (127) for the slash-in-command-name restriction, but that status can vary between bash versions/ports for the same restriction. Locking the exact code here could make the test flaky across environments.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Comment thread stable-patches/tests/rbash_test.patch Outdated
+
+ # For restricted operations, we expect non-zero exit code
+ if [ $exit_code -ne 0 ]; then
+ echo " ✓ PASS: Command properly restricted (exit code: $exit_code)"
Copy link
Copy Markdown

@augmentcode augmentcode Bot Apr 27, 2026

Choose a reason for hiding this comment

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

stable-patches/tests/rbash_test.patch:34: The ✓/✗ Unicode markers in .tests/.right may not round-trip cleanly in environments that do ASCII↔EBCDIC conversion, leading to diffs even when behavior is correct. If z/OS builds run through such conversions, this could create false failures.

Severity: low

Other Locations
  • stable-patches/tests/rbash_test.patch:37
  • stable-patches/tests/rbash_test.patch:71
  • stable-patches/tests/rbash_test.patch:74
  • stable-patches/tests/rbash_test.patch:87
  • stable-patches/tests/rbash_test.patch:90
  • stable-patches/tests/rbash_test.patch:102
  • stable-patches/tests/rbash_test.patch:108
  • stable-patches/tests/rbash_test.patch:114
  • stable-patches/tests/rbash_test.patch:117
  • stable-patches/tests/rbash_test.patch:120
  • stable-patches/tests/rbash_test.patch:123
  • stable-patches/tests/rbash_test.patch:126
  • stable-patches/tests/rbash_test.patch:133

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Comment thread stable-patches/MANIFEST.patch Outdated
tests/quotearray3.sub f
tests/quotearray4.sub f
tests/quotearray5.sub f
+tests/rbash.tests f
Copy link
Copy Markdown

@augmentcode augmentcode Bot Apr 27, 2026

Choose a reason for hiding this comment

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

stable-patches/MANIFEST.patch:9: The new MANIFEST entries use spaces before the f flag while surrounding entries are tab-delimited; if any tooling expects the exact tab format, these files might not be picked up. Worth matching the existing delimiter to avoid subtle packaging/test-list issues.

Severity: low

Other Locations
  • stable-patches/MANIFEST.patch:10
  • stable-patches/MANIFEST.patch:18

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Comment thread stable-patches/tests/rbash_test.patch Outdated
+
+ # For restricted operations, we expect non-zero exit code
+ if [ $exit_code -ne 0 ]; then
+ echo " ✓ PASS: Command properly restricted (exit code: $exit_code)"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just check on these characters in echo statements!
Augment has stated it in detail

Copy link
Copy Markdown
Member

@sachintu47 sachintu47 left a comment

Choose a reason for hiding this comment

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

LGTM

@sabi789 sabi789 closed this Apr 28, 2026
@sabi789 sabi789 reopened this Apr 28, 2026
sabi789 added 2 commits April 28, 2026 06:46
Signed-off-by: sabi789 <sabithac6298@gmail.com>
Signed-off-by: sabi789 <sabithac6298@gmail.com>
@sachintu47 sachintu47 merged commit 4f3baee into zopencommunity:main Apr 28, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants