Skip to content

Fix special characters in arguments - #51

Merged
gcbeltramini merged 12 commits into
mainfrom
gb/special-chars
Jul 6, 2026
Merged

Fix special characters in arguments#51
gcbeltramini merged 12 commits into
mainfrom
gb/special-chars

Conversation

@gcbeltramini

Copy link
Copy Markdown
Owner

Fix string injection:

# Normal behavior
❯ mycli hello world 'foo'
Hello from the CLI, foo :)
The flag '--some-flag' is 'false'
The named parameter '--foo' is '42'
If you can see this green message, it means that the CLI is working on your machine.

# Problem:
❯ mycli hello world '\n'
# <<-- docopt parsed arguments -->>
export foo="42"
export ls_args=()
export name="\n"
export some_flag="false"
export world="true"
# <<----------------------------->>

With the proposed changes in this PR:

❯ mycli hello world '\n'
Hello from the CLI,
 :)
The flag '--some-flag' is 'false'
The named parameter '--foo' is '42'
If you can see this green message, it means that the CLI is working on your machine.

@gcbeltramini
gcbeltramini requested a review from Copilot June 21, 2026 23:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a string-escape/injection issue in argument parsing by preventing backslash escapes (e.g., \n) from being interpreted as real newlines during the “should we eval this?” check, which previously could cause docopt output to be mis-parsed and unsafe evaluation decisions.

Changes:

  • Replace echo -e usage in _is_str_to_eval with a here-string pipeline to avoid interpreting escape sequences.
  • Update/extend tests to cover multi-line export blocks and literal \n content.
  • Tighten eval_args test expectations to validate actual exported values.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
core/helpers/cli_parse_args.sh Stops interpreting escape sequences while deciding whether docopt output is safe to eval.
tests/core/helpers/test_cli_parse_args.sh Adds/updates tests for multi-line export detection and literal backslash-n handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/helpers/cli_parse_args.sh Outdated
Comment thread tests/core/helpers/test_cli_parse_args.sh Outdated
gcbeltramini and others added 2 commits June 21, 2026 21:05
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread core/helpers/cli_parse_args.sh Outdated
Comment thread tests/core/helpers/test_cli_parse_args.sh
gcbeltramini and others added 2 commits June 21, 2026 21:21
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread core/helpers/cli_parse_args.sh Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread core/helpers/cli_parse_args.sh
Comment thread tests/core/helpers/test_cli_parse_args.sh
Error:
```
Every function in a helper file should have a corresponding test (test coverage = 100%)...
comm: file 1 is not in sorted order
comm: file 2 is not in sorted order
comm: input is not in sorted order
```

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@gcbeltramini
gcbeltramini merged commit d97969d into main Jul 6, 2026
2 checks passed
@gcbeltramini
gcbeltramini deleted the gb/special-chars branch July 6, 2026 00:03
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.

2 participants