Skip to content

feat(prompt): shell backtick substitution in PROMPT1/PROMPT2#748

Merged
NikolayS merged 6 commits intomainfrom
fix/744-prompt-backtick
Mar 27, 2026
Merged

feat(prompt): shell backtick substitution in PROMPT1/PROMPT2#748
NikolayS merged 6 commits intomainfrom
fix/744-prompt-backtick

Conversation

@NikolayS
Copy link
Copy Markdown
Owner

Closes #744

Implements backtick command substitution in PROMPT1/PROMPT2, matching psql behaviour.

Example: PROMPT1='hostname:%n@%m:%/> ' now renders with actual hostname output.

Tests added for: valid command, no trailing newline, failing command (empty), no backticks, multiple substitutions.

@NikolayS
Copy link
Copy Markdown
Owner Author

REV Review — fix/744-prompt-backtick

APPROVE

Clean, correct implementation matching psql semantics.

  • expand_prompt_backticks: char-by-char parser, handles nested backticks correctly, sh -c execution, graceful failure (empty string, no panic)
  • Applied before expand_prompt in build_prompt_from_settings — correct ordering, matches psql
  • Trailing newline trim handles LF and CRLF; minor: .trim_end_matches(|c| c == '\n' || c == '\r') would be slightly more idiomatic but current approach works fine
  • 5 tests cover all paths: valid command, no trailing newline, failing command, no backticks, multiple substitutions

Minor nit: expand_prompt_backticks is pub but only used in this module — pub(super) or pub(crate) would be tighter visibility. Not a blocker.

No SOC2 items. No new dependencies.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 25, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.17%. Comparing base (a0e8125) to head (bd86fbe).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #748      +/-   ##
==========================================
+ Coverage   69.11%   69.17%   +0.06%     
==========================================
  Files          47       47              
  Lines       32000    32062      +62     
==========================================
+ Hits        22115    22177      +62     
  Misses       9885     9885              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@NikolayS
Copy link
Copy Markdown
Owner Author

Code Review: PR #748 — feat(prompt): shell backtick substitution in PROMPT1/PROMPT2

Reviewer: Max
Date: 2026-03-25
PR: #748


Summary

Adds psql-compatible backtick command substitution to PROMPT1/PROMPT2. Implementation is clean and the test coverage covers the happy path well. A few issues worth addressing, one of which is a subtle but real correctness bug.


BLOCKING

B1 — Unterminated backtick silently drops content | Confidence: 8

If the prompt string contains an unmatched opening backtick (e.g. "user whoami> "), the parser enters the cmd` collection loop and consumes all remaining characters looking for a closing backtick that never comes. The collected string is then executed as a shell command, which is almost certainly wrong. The user likely intended the backtick to appear literally.

// Current: no check for unclosed backtick
for inner in chars.by_ref() {
    if inner == '`' { break; }
    cmd.push(inner);
}
// If loop exits without finding '`', cmd contains the rest of the string
// and gets executed as a shell command.

Fix: Track whether the closing backtick was found. If not, emit the opening backtick and the collected characters as-is (same as bash/psql behavior for unmatched backticks in non-interactive contexts — silently pass through or error).

let mut found_close = false;
for inner in chars.by_ref() {
    if inner == '`' { found_close = true; break; }
    cmd.push(inner);
}
if !found_close {
    result.push('`');
    result.push_str(&cmd);
    continue; // or break, depending on desired semantics
}

B2 — trim_end_matches trims all trailing \r and \n, not just one newline | Confidence: 7

The current trimming logic:

output.trim_end_matches('\n').trim_end_matches('\r')

This greedily strips all trailing newlines and all trailing carriage returns. A command that intentionally outputs multiple trailing newlines (rare but possible) will have them silently eaten. More importantly, the order is wrong for \r\n sequences: trim_end_matches('\n') first removes \n, then trim_end_matches('\r') removes \r, which works — but if the output ends in \r\n\r\n, both \r\n pairs are stripped rather than just the last one.

psql behavior strips the final newline only. The correct equivalent:

let trimmed = output.trim_end_matches(|c| c == '\r' || c == '\n');
// or more precisely, strip a single trailing \r\n or \n:
let trimmed = output.strip_suffix("\r\n")
    .or_else(|| output.strip_suffix('\n'))
    .unwrap_or(&output);

The practical impact is low, but this is a behavioral divergence from psql.


NON-BLOCKING

N1 — Security posture is correct but deserves a doc comment | Confidence: 6

sh -c <user-config-value> is intentional here (psql parity), and the threat model is appropriate: PROMPT1/PROMPT2 come from the user's own .rpgrc or session \set, so they already control the machine. This is not a privilege escalation vector.

However, the existing doc comment doesn't call this out explicitly. A one-liner noting "execution is intentional and matches psql; the prompt string is user-controlled config, not user input from queries" would help future readers who might flag this in a security audit.


N2 — Command inherits the parent process environment | Confidence: 5

std::process::Command::new("sh") inherits the full environment from the rpg process. This matches psql behavior, but it means things like $PGPASSWORD, $PGHOST, etc., are visible to the subshell command. Not a security issue given the threat model (user's own config), but worth a doc note for awareness.


N3 — No test for unclosed backtick (related to B1) | Confidence: 7

Once B1 is fixed, add a test:

#[test]
fn unclosed_backtick_passthrough() {
    assert_eq!(expand_prompt_backticks("user `whoami> "), "user `whoami> ");
}

N4 — No test for empty backtick pair | Confidence: 5

"`echo hello` ` ` — what does `` (empty backtick pair) do? Currently it runs sh -c "" which exits 0 with no output, producing an empty string. That's probably fine, but an explicit test documents the intent:

#[test]
fn empty_backtick_gives_empty() {
    assert_eq!(expand_prompt_backticks("``"), "");
}

N5 — Non-UTF8 command output silently discarded | Confidence: 4

.and_then(|o| String::from_utf8(o.stdout).ok())
.unwrap_or_default()

If the command outputs non-UTF8 bytes, the entire output is replaced with an empty string. This is consistent with the stated behavior ("produces no output → empty string"), but the doc comment doesn't mention it. Consider String::from_utf8_lossy if best-effort display of non-UTF8 is preferable, or keep the current behavior and document it.


POTENTIAL

P1 — build_prompt_from_settings calls backtick expansion on every prompt render | Confidence: 4

Prompts are re-rendered on every REPL iteration. If PROMPT1 contains a slow command (e.g., git branch --show-current in a large repo), this will introduce noticeable latency on every keypress/enter. psql has the same behavior, so it's not a bug — but it may surprise users. Consider noting this in the user-facing docs for the feature.


Test Coverage Assessment

Five tests cover: basic substitution, trailing newline stripping, command failure, no-backtick passthrough, and multiple substitutions. That's a solid baseline. Missing: unclosed backtick (see B1/N3), empty backtick pair (N4), backtick at string start/end edge cases.


Verdict

One genuine correctness bug (B1 — unmatched backtick executes garbage as a shell command), one minor behavioral divergence from psql (B2 — over-aggressive newline trimming). Both are straightforward to fix. The security posture is correct for the threat model. Tests are good but need the unclosed-backtick case once B1 is addressed.

@NikolayS
Copy link
Copy Markdown
Owner Author

Fixed REV B1: unterminated backtick now passes through literally instead of executing as shell command. Added test backtick_unterminated_passes_through_literally. All tests pass.

@NikolayS
Copy link
Copy Markdown
Owner Author

REV Code Review Report

CI Status CodeQL
❌ Lint FAILED / ✅ Tests passed ✅ Passed

BLOCKING ISSUES (1)

CRITICAL CI/Pipelinecargo fmt --check failed

rustfmt requires multi-argument assert_eq! to be formatted across multiple lines.
Diff shown by CI:

-        assert_eq!(result, "`unclosed", "unterminated backtick must pass through literally");
+        assert_eq!(
+            result, "`unclosed",
+            "unterminated backtick must pass through literally"
+        );

Fix: Run cargo fmt locally and push. One-liner assert_eq! with a message arg exceeds rustfmt's line width limit.
Job: Checks / Lint


NON-BLOCKING (0)

No non-blocking issues found.


POTENTIAL ISSUES (2)

MEDIUM src/repl/mod.rs — Missing test: empty backtick command (confidence: 6/10)

`` (empty command) is a valid input — sh -c "" exits 0 with no output. The current implementation handles it correctly (produces ""), but there is no test asserting this behavior. If the logic were changed, the regression would go undetected.
Suggestion: Add test backtick_empty_command_gives_empty asserting expand_prompt_backticks("``") returns "".

LOW src/repl/mod.rs — Windows-style \r\n stripping uses sequential trim (confidence: 5/10)

trim_end_matches('\n').trim_end_matches('\r') correctly strips \r\n endings (strips all trailing \n, then all trailing \r). However, it also strips any number of interleaved trailing \r and \n characters independently, which differs slightly from stripping only line endings. This matches psql behaviour and is unlikely to matter in practice, but there is no test documenting the intended behaviour for \r\n output.
Suggestion: Add a test for a command that produces "hello\r\n" output to document expected trimming behaviour.


Summary

Area Findings Potential Filtered
CI/Pipeline 1 0 0
Security 0 0 0
Bugs 0 0 0
Tests 0 2 0
Guidelines 0 0 0
Docs 0 0 0

Note:

  • Findings: High-confidence issues (8-10/10) — blocking or non-blocking per severity
  • Potential: Medium-confidence issues (4-7/10) — review manually
  • Filtered: Low-confidence issues (0-3/10) — excluded as likely false positives

REV-assisted review (AI analysis by postgres-ai/rev)

@NikolayS
Copy link
Copy Markdown
Owner Author

REV Code Review — PR #748

PR: #748 — feat(prompt): shell backtick substitution in PROMPT1/PROMPT2
Branch: fix/744-prompt-backtick
AI-Assisted: Yes


BLOCKING ISSUES

HIGH src/repl/mod.rs:196–210 — Shell injection via backtick command execution (confidence: 9/10)

expand_prompt_backticks passes the raw content of backtick-delimited segments directly to sh -c. PROMPT1/PROMPT2 are user-controlled config values, which means anyone who can write the config file can execute arbitrary shell commands. This is equivalent to eval. While psql does the same thing (backtick substitution runs shell commands), rpg should document this behavior explicitly and ensure it is gated behind an explicit user opt-in or at minimum documented as a security consideration in the settings/config docs. At a minimum, add a warning comment in the code and in the config documentation that backtick expansion executes shell commands.

Fix: Add a prominent comment: // Security note: executes arbitrary shell via sh -c; source is user config only. Also add a line to the relevant config documentation. Do not silently shell-exec without any documentation.


NON-BLOCKING

LOW src/repl/mod.rs:213–216 — Unterminated backtick passes through result without the trailing content after the unclosed backtick

When there is no closing backtick, the code emits the literal backtick and then the collected cmd chars. This is correct and consistent with psql behavior. The test backtick_unterminated_passes_through_literally validates it. No action needed.

LOW src/repl/mod.rs:421 — Backtick expansion applied to template before %-sequence expansion

The comment says this matches psql behavior. Confirm that psql indeed does backtick substitution before %-expansion (it does — documented in psql source). Comment is correct.


POTENTIAL ISSUES

MEDIUM src/repl/mod.rs:205 — Shell commands spawned synchronously on every prompt render (confidence: 7/10)

Prompts are re-rendered on every command. If a backtick command is slow (e.g., a network call), it will block the REPL. psql has the same issue. Consider documenting this limitation; users should keep prompt commands fast. No code change required unless you want to add a timeout.

MEDIUM src/repl/mod.rs:209trim_end_matches('\n').trim_end_matches('\r') — trailing CR/LF stripped but leading whitespace preserved (confidence: 5/10)

psql trims only trailing newline. If a command outputs leading spaces they are preserved. This matches psql. Verify this is intentional and document it if so.


Summary

Area Findings Potential Filtered
Security 1 0 0
Bugs 0 0 0
Tests 0 1 0
Guidelines 0 0 0
Docs 1 0 0

6 unit tests cover the main scenarios. The HIGH finding is not a runtime bug — the feature works as intended — but the shell execution behavior is undocumented and security-relevant. CI is green.


REV-assisted review (AI analysis by postgres-ai/rev)

@NikolayS
Copy link
Copy Markdown
Owner Author

REV

PR #748 — feat(prompt): shell backtick substitution in PROMPT1/PROMPT2

Summary: Adds expand_prompt_backticks() which replaces backtick-delimited commands in prompt strings with trimmed stdout of sh -c cmd. Applied before expand_prompt() in build_prompt_from_settings(), matching psql behavior.

Observations:

  1. Security note — Executes arbitrary shell commands on every prompt render. Consistent with psql, but a comment noting the footgun would help: user pastes untrusted PROMPT1 → code runs. Same issue as psql; just flag it.

  2. Perf: called on every prompt render — Shell fork per prompt tick is noticeable with slow commands (e.g. git branch in large repos). Caching with short TTL is out of scope here but worth the backlog.

  3. Unterminated backtick behavior — passes through literally. Test covers it. Matches psql. Correct.

  4. Failing command → empty string — correct. Command::new("sh") also fails if sh not on PATH (rare but possible in containers). Empty fallback is fine.

  5. Nested backticks not supported — consistent with psql.

  6. trim_end_matches('\n').trim_end_matches('\r') — strips all trailing newlines, not just one. psql chomp strips exactly one. Probably fine, but worth a comment noting the divergence.

Verdict: Correct implementation, tests cover all edge cases. Points 1 and 6 worth a quick look before merge but neither is a blocker.

@NikolayS
Copy link
Copy Markdown
Owner Author

REV

#748 — feat(prompt): shell backtick substitution in PROMPT1/PROMPT2

Solid. Matches psql behavior, good test coverage, handles edge cases cleanly.

What's good

  • The backtick-before-%-expansion ordering is correct and matches psql
  • Unterminated backtick pass-through is the right call (no silent data loss)
  • 6 tests cover the main paths: echo, no trailing newline, failing command, no backticks, multiple, unterminated
  • trim_end_matches('\n').trim_end_matches('\r') handles both LF and CRLF (though trim_end_matches(|c| c == '\n' || c == '\r') is slightly cleaner)

Concerns

  1. Shell injection via PROMPT1 — this executes arbitrary shell commands from the prompt string. If someone sets PROMPT1 via environment variable or config file that's world-writable, this is a shell injection vector. psql has the same behavior so parity is fine, but it should be documented. A note in the relevant config docs or --help output would be appropriate.

  2. Command timeoutstd::process::Command has no timeout. A sleep 30 in PROMPT1 will hang the REPL for 30 seconds. psql has the same limitation. Acceptable for now, but worth a // TODO: add timeout comment.

  3. UTF-8 assumptionString::from_utf8(o.stdout).ok() silently drops non-UTF-8 output. Most shell output is UTF-8 but if someone runs locale or similar with a non-UTF-8 locale this silently substitutes empty string. String::from_utf8_lossy would be more user-friendly.

  4. chars().peekable() but .peek() is never called — the iterator is .peekable() but you only use .next() and .by_ref(). Drop .peekable() or use it.

Minor

  • Test backtick_failing_command_gives_empty: nonexistent_cmd_xyz_123 might actually exist on some exotic system. Not realistic, but a more obviously invalid command like /bin/this_does_not_exist_42 is slightly more robust.

Verdict: approve with nits. Items 3 and 4 are worth a quick fix; 1 and 2 are acceptable known limitations.

@NikolayS
Copy link
Copy Markdown
Owner Author

REV Code Review — PR #748: feat(prompt): shell backtick substitution in PROMPT1/PROMPT2

CI: green | Diff scope: src/repl/mod.rs


Blocking findings

None.


Non-blocking findings

[SECURITY / MEDIUM] Arbitrary shell command execution via PROMPT1/PROMPT2

expand_prompt_backticks runs any shell command found inside backticks using sh -c. If PROMPT1 is read from a .rpg.toml or .psqlrc file that was placed by a third party (e.g., in a shared directory or repository), this is arbitrary code execution. This matches psql behavior exactly, so it is not a regression, but it is worth a doc comment noting the security implication — users who set prompts from untrusted config files should be aware.

[BUG / LOW] Unterminated backtick handling emits the backtick but drops the command content

result.push('`');
result.push_str(&cmd);
continue;

The test backtick_unterminated_passes_through_literally verifies `unclosed`unclosed. Good. However, continue here skips the outer while loop's next iteration correctly — but note that cmd at this point contains everything after the opening backtick until end of string. The result is `unclosed which matches the test. This is correct. The only edge case not tested: a backtick at the very end of string (empty cmd) — ` alone. This produces ` (correct), but a test for it would be reassuring.

[BEHAVIOR / LOW] Trailing newlines: only \n and \r are stripped, not \r\n pairs

output.trim_end_matches('\n').trim_end_matches('\r')

This strips any number of trailing \n, then any number of trailing \r. A Windows command that outputs hello\r\n would produce hello (correct). But hello\r\n\r\n would strip all four characters, which is the desired behavior (all trailing whitespace from the shell). An alternative: output.trim_end_matches(|c| c == '\n' || c == '\r') or just output.trim_end(). The current approach is fine for Unix; acceptable for the use case.

[STYLE / LOW] chars.by_ref() inside the inner loop

Using for inner in chars.by_ref() is idiomatic and correct — it advances the outer iterator. This is the right approach. No issue; noting for completeness that this is well done.


Potential issues (low confidence)

[BEHAVIOR] Nested backticks are not supported

Backtick nesting (e.g., `echo `hostname ``) is not handled — the inner backtick would close the outer command prematurely, resulting in sh -c "echo " being executed followed by `hostname` being literal. This matches psql behavior (which also does not support nested backticks), so it is not a regression. A comment noting this limitation would help future readers.


Summary

Minimal, correct implementation that matches psql semantics. Test coverage covers the key cases. The security note about shell execution from config files is the main item worth documenting. No blockers.

@NikolayS
Copy link
Copy Markdown
Owner Author

REV Code Review — PR #748 (re-review after fixes)

Branch: fix/744-prompt-backtick
Diff: 4 commits, 2 files changed (+67/-2)
Agents: Security, Bug Hunter, Test Analyzer, Guidelines Checker, Docs Reviewer


SECURITY REVIEWER

Finding 1

FINDING:
- severity: MEDIUM
- confidence: 6
- file: src/repl/mod.rs
- line: 219
- issue: String::from_utf8 silently drops non-UTF-8 output
- evidence: .and_then(|o| String::from_utf8(o.stdout).ok())
- fix: Use String::from_utf8_lossy(&o.stdout).into_owned() — this is what
       the rest of the codebase already does (line 3166). The current code
       silently returns "" if the shell command emits non-UTF-8 bytes, which
       is confusing but not a security issue per se. Low severity, but
       inconsistent with the rest of the codebase.
- cwe: N/A

Finding 2

FINDING:
- severity: LOW
- confidence: 5
- file: src/repl/mod.rs
- line: 213-222
- issue: Shell command runs without timeout — a prompt like `sleep 99` or
         `find /` would block the REPL indefinitely every keystroke
- evidence: std::process::Command::new("sh").arg("-c").arg(&cmd).output()
- fix: psql itself does not set a timeout either, so this matches the
       spec's "psql-compatible" goal. Worth a TODO comment; not blocking
       given explicit psql parity requirement.
- cwe: CWE-400 (resource exhaustion)

No other security findings. Shell commands come exclusively from user-set PROMPT1/PROMPT2 vars — the security note in the docstring covers this well.


BUG HUNTER

Finding 3

FINDING:
- severity: LOW
- confidence: 7
- file: src/repl/mod.rs
- line: 221
- issue: Newline stripping order can leave \n if output ends with \n\r
         (unusual CRLF variant)
- evidence: output.trim_end_matches('\n').trim_end_matches('\r')
            For "hello\n\r": trim_end('\n') is a no-op (rightmost char is \r),
            then trim_end('\r') removes \r → leaves "hello\n"
- fix: Use output.trim_end_matches(['\n', '\r']) which strips any combination
       of trailing CR/LF in any order (same pattern used at line 2401).
       Or simply: output.trim_end_matches('\n').trim_end_matches('\r')
       → output.trim_end_matches(|c| c == '\n' || c == '\r')
       In practice sh -c on Unix always produces \n endings, so this is a
       theoretical bug. Still worth fixing for correctness.

No other bugs. Logic flow, iterator usage, found_close sentinel, and error fallback all look correct.


TEST ANALYZER

Finding 4

FINDING:
- severity: LOW
- confidence: 7
- file: src/repl/mod.rs
- lines: 6312-6357
- issue: Missing test for output with CRLF line endings (\r\n)
- fix: Add: assert_eq!(expand_prompt_backticks("`printf 'hello\r\n'`"), "hello");
       Also missing: empty backtick pair `\`\`` — psql runs "" as a shell
       command (sh -c "" exits 0 with empty output). Current code handles
       this correctly (cmd="" → output="") but it's not tested.

Finding 5

FINDING:
- severity: INFO
- confidence: 6
- file: src/repl/mod.rs
- issue: No integration test for build_prompt_from_settings with a backtick
         in PROMPT1. The unit tests cover expand_prompt_backticks in
         isolation but not the full pipeline (backtick → %-expand).
- fix: Add a test like:
       settings.vars.set("PROMPT1", "`echo db`/%R%x%# ");
       // verify it produces "db/=> "

Coverage is otherwise solid: happy path, multi-expand, unterminated backtick, failing command, no-backtick passthrough. Good test naming and isolation.


GUIDELINES CHECKER

Finding 6

FINDING:
- severity: INFO
- confidence: 8
- file: src/repl/mod.rs
- line: 221
- issue: Minor style: trim_end_matches pattern inconsistency
- rule: Codebase convention (see line 2401: trim_end_matches(['\r', '\n']))
- suggestion: Use trim_end_matches(['\n', '\r']) for consistency with
              the rest of the file.

Commit messages comply with Conventional Commits. All 4 commits are well-scoped: feat(prompt):, fix(prompt):, fix(fmt):, docs(prompt):. Subject lines are under 50 chars. ✓


DOCS REVIEWER

Finding 7

FINDING:
- severity: INFO
- confidence: 7
- file: src/repl/mod.rs
- lines: 176-191
- issue: Docstring says "substitutes an empty string" for both command
         failure AND non-UTF8 output — these are conflated. The non-UTF8
         silent-drop behavior (String::from_utf8(...).ok()) is a separate
         case worth mentioning, or better: switch to from_utf8_lossy and
         remove the ambiguity.

The # Security note section in the docstring is well-written — explicit about trust boundary. No other doc issues.

SPEC.md changes are accurate and appropriately scoped — backtick behavior documented in both FR-2 (feature) and the future TODO checklist.


Summary

# Severity Area Finding
1 MEDIUM Bug/Consistency from_utf8 should be from_utf8_lossy
2 LOW Reliability No timeout on shell command execution
3 LOW Bug Newline trim order can leave \n for \n\r endings
4 LOW Tests No CRLF test; no empty-backtick test
5 INFO Tests No integration test for full backtick→%-expand pipeline
6 INFO Style Trim pattern inconsistency vs rest of codebase
7 INFO Docs Conflated empty-string cases in docstring

BLOCKING findings: NO

All findings are LOW/MEDIUM/INFO. The primary fix worth making before merge is Finding 3 (newline trim order — easy one-liner) combined with Finding 1 (from_utf8_lossy for consistency). Everything else is polish.

The core implementation is correct, psql-compatible, well-documented, and the unterminated-backtick fix from the previous review is properly handled. PASS.


REV automated review — agents: Security (Opus), Bug Hunter (Opus), Test Analyzer (Sonnet), Guidelines (Sonnet), Docs (Sonnet)

@NikolayS
Copy link
Copy Markdown
Owner Author

CI is green and REV review found no blocking findings. This PR is ready for merge — awaiting Nik's approval.

@NikolayS
Copy link
Copy Markdown
Owner Author

REV

PR #748 — feat(prompt): shell backtick substitution in PROMPT1/PROMPT2

Clean implementation, matches psql behavior, good test coverage.

Observations:

  1. sh -c is synchronous and blocking — if the backtick command hangs (e.g. hostname -f on a misconfigured DNS), it will block the prompt render indefinitely. psql has the same flaw, but we could add a timeout (Command::timeout via wait_with_output + a thread, or just document it as a known limitation). Worth at least a // TODO: add timeout comment.

  2. Security note is accurate and well-written — good that it's explicit. One small addition: the note says "never from query input or remote data" but it might be worth mentioning .rpg.toml specifically, since that's where PROMPT1/PROMPT2 are configured. That's the actual trust boundary.

  3. expand_prompt_backticks applied before expand_prompt — the comment says "shell output can itself contain %-sequences", which is correct and matches psql. Confirmed in build_prompt_from_settings.

  4. Tests cover the failure casebacktick_failing_command_gives_empty is the right test. backtick_unterminated_passes_through_literally is also the right behavior and well-tested.

  5. SPEC.md update — the spec now documents backtick behavior in two places (FR-2 and the prompt format codes section). Both are consistent.

No blockers. The timeout concern (#1) is a quality-of-life issue, not a correctness issue.

@NikolayS
Copy link
Copy Markdown
Owner Author

Testing Evidence — Shell Backtick Substitution in PROMPT1/PROMPT2

Branch: fix/744-prompt-backtick
Binary: rpg 0.8.3 (21b2c29, built 2026-03-26)
Server: PostgreSQL 17.7 on 127.0.0.1:15433


Build

$ cd /tmp/rpg-pr-748 && ~/.cargo/bin/cargo build 2>&1 | tail -3
   Compiling rpg v0.8.3 (/tmp/rpg-pr-748)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 3m 55s

$ ./target/debug/rpg --version
rpg rpg 0.8.3 (21b2c29, built 2026-03-26)

Test 1: PROMPT1 with whoami

Default prompt, then \set PROMPT1 '\whoami`@%/ # '`:

ashtest=# \set PROMPT1 '`whoami`@%/ # '
tars@ashtest # SELECT current_user, current_database(), version();
 current_user | current_database |                                                      version
--------------+------------------+--------------------------------------------------------------------------------------------------------------------
 postgres     | ashtest          | PostgreSQL 17.7 (Debian 17.7-3.pgdg13+1) on x86_64-pc-linux-gnu, compiled by gcc (Debian 14.2.0-19) 14.2.0, 64-bit
(1 row)
tars@ashtest #

The backtick command whoami was evaluated at prompt render time: tars (the OS user) appears in the prompt.


Test 2: PROMPT1 with date +%H:%M:%S

tars@ashtest # \set PROMPT1 '`date +%H:%M:%S`> '
06:50:49> SELECT now();
              now
-------------------------------
 2026-03-26 06:50:54.732888+00
(1 row)
06:50:54>

The timestamp updates on every prompt render — 06:50:49 before the query, 06:50:54 after it returned.


Test 3: PROMPT2 with date +%H:%M:%S (continuation prompt)

06:50:54> \set PROMPT2 '`date +%H:%M:%S`... '
06:51:03> SELECT
06:51:03...   'backtick_substitution_works' AS feature_status;
       feature_status
-----------------------------
 backtick_substitution_works
(1 row)
06:51:10>

PROMPT2 also evaluates backtick substitution live — 06:51:03... during multiline input, 06:51:10 after completion.


Verdict

✅ PROMPT1 backtick substitution works — shell command output rendered at each prompt
✅ PROMPT2 backtick substitution works — continuation prompt also evaluates correctly
✅ Dynamic update — timestamp changes between prompts, not evaluated once at \set time
✅ Mixed format specifiers work — whoami@%/ # combines shell output with rpg's own %/ (database name)

Add regression tests confirming that content after an unterminated
backtick is not silently dropped.

The existing expand_prompt_backticks() implementation already handles
this correctly via the found_close guard — but only one test existed
for the unterminated case (`unclosed), which did not cover the actual
reported bug pattern: 'hello `date world' where 'world' would be lost.

Two new tests added:
- backtick_unterminated_preserves_content_after: the exact bug from #744
- backtick_unterminated_with_prefix_and_suffix: prefix + suffix variant
@NikolayS
Copy link
Copy Markdown
Owner Author

CI is green and REV review found no blocking findings. Testing evidence posted. This PR is ready for merge — awaiting Nik's approval.

@NikolayS NikolayS merged commit 02fe327 into main Mar 27, 2026
16 checks passed
@NikolayS NikolayS deleted the fix/744-prompt-backtick branch March 27, 2026 23:26
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.

feat(prompt): support shell scripts in PROMPT1/PROMPT2 (psql parity)

2 participants