test: regression test for -- end-of-options separator in _write_to_journal#122
Merged
GingerGraham merged 2 commits intoJun 6, 2026
Merged
Conversation
Adds test_write_to_journal_passes_double_dash_before_message to test_journal_logging.sh to assert that log_to_journal passes -- before the message argument when calling logger, preventing option-injection for messages that begin with a hyphen (e.g. '-n not-an-option'). Uses assert_contains on the captured stub output instead of assert_file_contains to avoid a grep quirk where patterns starting with '--' are misinterpreted as long option names on this platform. Closes review comment #121 (comment)
Copilot
AI
changed the title
[WIP] Fix code based on review comment 3368180009
test: regression test for Jun 6, 2026
-- end-of-options separator in _write_to_journal
GingerGraham
approved these changes
Jun 6, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds regression coverage to ensure _write_to_journal always uses the -- end-of-options separator before the message argument, preventing logger option-injection when messages begin with -.
Changes:
- Adds
test_write_to_journal_passes_double_dash_before_messageto validate the stubbedloggerinvocation contains--immediately before a hyphen-prefixed message. - Updates the test suite “Covers” header and ensures the new regression test runs with the rest of the journal logging suite.
GingerGraham
added a commit
that referenced
this pull request
Jun 6, 2026
* fix(#120): ensure message handling in journal logging command * test: regression test for `--` end-of-options separator in `_write_to_journal` (#122) * Initial plan * test: add regression test for -- separator in _write_to_journal Adds test_write_to_journal_passes_double_dash_before_message to test_journal_logging.sh to assert that log_to_journal passes -- before the message argument when calling logger, preventing option-injection for messages that begin with a hyphen (e.g. '-n not-an-option'). Uses assert_contains on the captured stub output instead of assert_file_contains to avoid a grep quirk where patterns starting with '--' are misinterpreted as long option names on this platform. Closes review comment #121 (comment) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
_write_to_journalfix for option-injection (messages starting with-) had no regression coverage, leaving the behaviour free to silently break.Changes
tests/test_journal_logging.sh— addstest_write_to_journal_passes_double_dash_before_message: invokeslog_to_journal INFO '-n not-an-option'via a stub logger and asserts the captured invocation contains-- -n, confirming the--separator always precedes the message argument in theloggercall:Uses
assert_containson$(cat "$STUB_CAPTURE")rather thanassert_file_containsto avoid agrepquirk where patterns beginning with--are misinterpreted as long option names.