Skip to content

[Detail Bug] CLI translation: sudo and silent modifiers generate invalid Bash due to missing spaces #1051

@detail-app

Description

@detail-app

Summary

  • Context: The Command module is responsible for translating Amber commands (e.g., $echo "hi"$) into Bash code, including handling modifiers like sudo, silent, and suppress.
  • Bug: The translation logic joins the sudo prefix, the command body, and the redirection modifiers without ensuring there are spaces between them.
  • Actual vs. expected: For a command like sudo $echo "hi"$, it generates ${__sudo}echo "hi" instead of ${__sudo} echo "hi". For silent $echo 1$, it generates echo 1>/dev/null 2>&1 instead of echo 1 >/dev/null 2>&1.
  • Impact: The sudo modifier is completely broken because it tries to execute a concatenated command name (e.g., sudoecho) which doesn't exist. The silent modifier can cause incorrect behavior when the command ends with a digit, as Bash interprets it as file descriptor redirection (e.g., 1>/dev/null redirects stdout).

Code with bug

// src/modules/command/cmd.rs

00097|         let translation = fragments!(sudo_prefix, translation, suppress, silent); // <-- BUG 🔴 Missing spaces between components

Evidence

1. Reproduction Test

I created a test case that specifically checks for the presence of spaces in the generated Bash code.

// src/tests/repro_sudo.rs

    #[test]
    fn test_translate_with_sudo_space() {
        let code = r#"
main {
    sudo $echo "test"$?
}
"#;
        let result = translate_amber_code(code).expect("Couldn't translate Amber code");
        println!("Result: {}", result);
        assert!(
            result.contains("${__sudo} echo"),
            "Output should contain sudo followed by a space and echo, but got: {}",
            result
        );
    }

Test Output:

---- tests::repro_sudo::tests::test_translate_with_sudo_space stdout ----
Result: ${__sudo}echo "test"
...
thread 'tests::repro_sudo::tests::test_translate_with_sudo_space' panicked at src/tests/repro_sudo.rs:29:9:
Output should contain sudo followed by a space and echo, but got: ${__sudo}echo "test"

2. Concatenation logic in fragments!

The fragments! macro in src/translate/fragments/mod.rs uses ListFragment::new() which defaults to an empty string separator.

// src/translate/fragments/list.rs

00018|     pub fn new(value: Vec<FragmentKind>) -> Self {
00019|         ListFragment {
00020|             values: value,
00021|             separator: ListFragmentSeparator::Empty, // <-- Separator is empty by default
00022|         }
00023|     }

3. Digit redirection bug with silent

If a command ends in a digit (like echo 1), the silent modifier (which expands to >/dev/null 2>&1 without a leading space) will cause Bash to interpret the digit as a file descriptor.

Reproduction Test:

    #[test]
    fn test_translate_with_silent_space() {
        let code = r#"
main {
    silent $echo 1$?
}
"#;
        let result = translate_amber_code(code).expect("Couldn't translate Amber code");
        assert!(result.contains("echo 1 >/dev/null"), "Output should contain a space before >/dev/null, but got: {}", result);
    }

Result: echo 1>/dev/null 2>&1 (Fails because Bash interprets 1> as redirecting stdout, but if the command was echo 2, echo 2>/dev/null would redirect stderr and print nothing to stdout, which is incorrect for echo 2).

Why has this bug gone undetected?

Existing tests for sudo (like test_translate_with_sudo in src/tests/compiling.rs) only check if the output contains "sudo", but not if it's correctly separated from the command. Most commands used in tests don't end in digits, so the silent redirection bug is also easily missed.

Recommended fix

Use ListFragment::new().with_spaces() to ensure all components are separated by spaces, or explicitly add spaces in the fragments! call.

// src/modules/command/cmd.rs

        let translation = ListFragment::new(vec![sudo_prefix, translation, suppress, silent])
            .with_spaces() // <-- FIX 🟢 
            .to_frag();

History

This bug was introduced in commit 82e51f1 (@callframe, 2026-02-17, PR #1017). During a refactor to support TextPart and fix string interpolation issues, the Command::translate method was updated to use the fragments! macro instead of ListFragment::new(...).with_spaces(), which caused command components to be concatenated without necessary separating spaces.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingdetail

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions