Skip to content

Commit 9c943d6

Browse files
authored
Merge pull request #1879 from rtk-ai/develop
Next Release
2 parents 1e0d01b + 8564ddc commit 9c943d6

48 files changed

Lines changed: 2638 additions & 785 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CONTRIBUTING.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ Don't invent new output formats. Don't add RTK-specific headers or markers in th
5050

5151
If a filter fails, fall back to raw output. RTK should never prevent a command from executing or producing output. Better to pass through unfiltered than to error out. Same for hooks: exit 0 on all error paths so the agent's command runs unmodified.
5252

53-
Every filter needs a fallback path. Every hook must handle malformed input gracefully.
53+
Every filter needs a fallback path. Every hook must handle malformed input gracefully. Truncation follows the same rule: capping output at N items is only acceptable if accompanied by a hint that lets the agent recover the hidden data.
5454

5555
### Zero Overhead
5656

@@ -262,6 +262,7 @@ cargo fmt --all --check && cargo clippy --all-targets && cargo test
262262
- [ ] Unit tests added/updated for changed code
263263
- [ ] Snapshot tests reviewed (`cargo insta review`)
264264
- [ ] Token savings >=60% verified
265+
- [ ] Any truncated list has a recovery hint (`force_tee_tail_hint` or `force_tee_hint`) and uses a `CAP_*` from `src/core/truncate.rs`
265266
- [ ] Edge cases covered
266267
- [ ] `cargo fmt --all --check && cargo clippy --all-targets && cargo test` passes
267268
- [ ] Manual test: run `rtk <cmd>` and inspect output

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
<p align="center">
1010
<a href="https://github.com/rtk-ai/rtk/actions"><img src="https://github.com/rtk-ai/rtk/workflows/Security%20Check/badge.svg" alt="CI"></a>
1111
<a href="https://github.com/rtk-ai/rtk/releases"><img src="https://img.shields.io/github/v/release/rtk-ai/rtk" alt="Release"></a>
12-
<a href="https://opensource.org/licenses/MIT"><img src="https://img.shields.io/badge/License-MIT-yellow.svg" alt="License: MIT"></a>
12+
<a href="https://opensource.org/licenses/Apache-2.0"><img src="https://img.shields.io/badge/License-Apache_2.0-blue.svg" alt="License: Apache 2.0"></a>
1313
<a href="https://discord.gg/RySmvNF5kF"><img src="https://img.shields.io/discord/1470188214710046894?label=Discord&logo=discord" alt="Discord"></a>
1414
<a href="https://formulae.brew.sh/formula/rtk"><img src="https://img.shields.io/homebrew/v/rtk" alt="Homebrew"></a>
1515
</p>
@@ -483,7 +483,7 @@ Join the community on [Discord](https://discord.gg/RySmvNF5kF).
483483

484484
## License
485485

486-
MIT License - see [LICENSE](LICENSE) for details.
486+
Apache License 2.0 - see [LICENSE](LICENSE) for details.
487487

488488
## Disclaimer
489489

install.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,13 @@ install() {
9898
error "Failed to download binary"
9999
fi
100100

101+
# Verify archive contents before extraction (CWE-22 path traversal).
102+
# Reject any entry with an absolute path or a ".." component.
103+
info "Verifying archive..."
104+
if tar -tzf "$ARCHIVE" | grep -qE '^/|(^|/)\.\.(/|$)'; then
105+
error "Archive contains unsafe paths (absolute or directory traversal) — refusing to extract"
106+
fi
107+
101108
info "Extracting..."
102109
tar -xzf "$ARCHIVE" -C "$TEMP_DIR"
103110

scripts/test-install.sh

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
#!/usr/bin/env sh
2+
# Tests for install.sh path traversal check (issue #1250, CWE-22).
3+
#
4+
# Verifies:
5+
# 1. Safe archives (single binary, "./prefix", subdirs) are accepted.
6+
# 2. Archives with absolute paths are rejected pre-extraction.
7+
# 3. Archives with ".." components are rejected pre-extraction.
8+
# 4. The check is still present in install.sh (regression guard).
9+
10+
set -eu
11+
12+
REPO_ROOT=$(cd "$(dirname "$0")/.." && pwd)
13+
INSTALL_SH="$REPO_ROOT/install.sh"
14+
15+
if [ ! -f "$INSTALL_SH" ]; then
16+
echo "FAIL: install.sh not found at $INSTALL_SH"
17+
exit 1
18+
fi
19+
20+
if ! command -v python3 >/dev/null 2>&1; then
21+
echo "SKIP: python3 not available — crafted tarball tests require python3"
22+
exit 0
23+
fi
24+
25+
TMPDIR=$(mktemp -d)
26+
trap 'rm -rf "$TMPDIR"' EXIT
27+
28+
# The check replicated from install.sh (keep in sync with install.sh).
29+
# Returns 0 when archive is safe, 1 when unsafe.
30+
check_archive() {
31+
if tar -tzf "$1" | grep -qE '^/|(^|/)\.\.(/|$)'; then
32+
return 1
33+
fi
34+
return 0
35+
}
36+
37+
# --- Build safe archive using standard tar ---
38+
mkdir -p "$TMPDIR/safe_src"
39+
printf '#!/bin/sh\necho rtk\n' > "$TMPDIR/safe_src/rtk"
40+
(cd "$TMPDIR/safe_src" && tar -czf "$TMPDIR/safe.tgz" rtk)
41+
42+
# --- Build crafted malicious archives with python ---
43+
python3 - "$TMPDIR" <<'PY'
44+
import sys, tarfile, io
45+
46+
base = sys.argv[1]
47+
48+
49+
def make(name, entry):
50+
with tarfile.open(f"{base}/{name}", "w:gz") as t:
51+
info = tarfile.TarInfo(name=entry)
52+
data = b"pwned"
53+
info.size = len(data)
54+
t.addfile(info, io.BytesIO(data))
55+
56+
57+
make("traversal.tgz", "../etc/evil")
58+
make("absolute.tgz", "/tmp/evil_abs")
59+
make("middle.tgz", "rtk/../../../etc/evil")
60+
make("end_dotdot.tgz", "rtk/..")
61+
PY
62+
63+
FAIL=0
64+
pass() { printf ' PASS: %s\n' "$1"; }
65+
fail() { printf ' FAIL: %s\n' "$1"; FAIL=1; }
66+
67+
echo "==> Functional checks"
68+
69+
if check_archive "$TMPDIR/safe.tgz"; then
70+
pass "safe archive accepted"
71+
else
72+
fail "safe archive rejected (false positive)"
73+
fi
74+
75+
for bad in traversal absolute middle end_dotdot; do
76+
if check_archive "$TMPDIR/$bad.tgz"; then
77+
fail "$bad archive accepted (should be rejected)"
78+
else
79+
pass "$bad archive rejected"
80+
fi
81+
done
82+
83+
echo "==> Regression guard"
84+
85+
if grep -qF 'tar -tzf' "$INSTALL_SH" && grep -qF '\.\.' "$INSTALL_SH"; then
86+
pass "install.sh still contains the path-traversal check"
87+
else
88+
fail "install.sh is missing the path-traversal check — was it removed?"
89+
fi
90+
91+
echo ""
92+
if [ "$FAIL" -eq 0 ]; then
93+
echo "All install.sh path traversal tests passed"
94+
exit 0
95+
else
96+
echo "Some tests failed"
97+
exit 1
98+
fi

src/cmds/README.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,21 @@ When filtering fails, fall back to raw output and warn on stderr. Never block th
252252

253253
Modules that parse structured output (JSON, NDJSON, state machines) must call `tee::tee_and_hint()` so users can recover full output on failure.
254254

255+
### Internal Truncation Recovery
256+
257+
When a filter caps a list at N items (e.g. `take(20)`), the remaining items must be accessible via a tee hint. **Never show `"… +N more"` without a recovery path** — the agent has no way to retrieve the hidden content.
258+
259+
**Choosing the right hint:**
260+
261+
| Content type | Function | Condition |
262+
|---|---|---|
263+
| Flat list — one item = one line in the tee | `force_tee_tail_hint(content, slug, MAX + 1)` | PR lists, error lines, file paths — anything where each item is a single-line string |
264+
| Multi-line blocks | `force_tee_hint(content, slug)` | Test failures, build error blocks — items that span multiple lines so a line offset is meaningless |
265+
266+
**Cap values come from `src/core/truncate.rs`.** Pick the `CAP_*` matching your data class (`CAP_ERRORS`, `CAP_WARNINGS`, `CAP_LIST`, `CAP_INVENTORY`) and bind it to a local `const MAX_XXX: usize = CAP_Y;`. Derive `take(MAX_XXX)`, `> MAX_XXX`, and the offset `MAX_XXX + 1` from the local. These CAPs will later become the configuration surface for per-filter cap tuning (user-overridable via config) — keep all truncation values routed through them so that hook lands as a single switch rather than a codebase-wide hunt. A filter that genuinely needs to deviate uses **`truncate::reduced(CAP_Y, n)`** (e.g. `reduced(CAP_WARNINGS, 5)`) so it still tracks the global when reconfigured — never a bare literal, never `cap - n` (underflows once caps are runtime-configurable), and never `*`/`/` (those scale unboundedly). `reduced` falls back to the full cap if the reduction would empty the list. Each deviation needs a one-line comment stating why; if there's no real reason, just use the plain CAP. See `src/core/README.md` ("Truncation Caps") for the full rationale.
267+
268+
**The tee content must match what `tail` produces.** For `force_tee_tail_hint`, build the tee from the same formatted values shown in the output — not raw/intermediate data. If the filter reformats items before displaying them, pre-build a `Vec<String>` of formatted lines and use it for both the display loop and the tee.
269+
255270
### Stderr Handling
256271

257272
Modules must capture stderr and include it in the raw string passed to `timer.track()`, so token savings reflect total output.
@@ -278,6 +293,7 @@ Adding a new filter or command requires changes in multiple places. For TOML-vs-
278293
- Use `RunOptions::default()` when filtering combined text output
279294
- Add `.tee("label")` when the filter parses structured output (enables raw output recovery on failure)
280295
- **Exit codes**: handled automatically by `run_filtered()` — just return its result
296+
- **Truncation**: if the filter caps any list at N items, emit `force_tee_tail_hint` (flat lists) or `force_tee_hint` (multi-line blocks) so the agent can recover hidden items — see [Internal Truncation Recovery](#internal-truncation-recovery). Use a named constant for the cap; derive the offset from it (`MAX_XXX + 1`)
281297
2. **Register module**:
282298
- Ecosystem `mod.rs` files use `automod::dir!()` — any `.rs` file in the directory becomes a public module automatically. No manual `pub mod` needed, but be aware: WIP or helper files will also be exposed. Only commit command-ready modules.
283299
- Add variant to `Commands` enum in `main.rs` with `#[arg(trailing_var_arg = true, allow_hyphen_values = true)]`

src/cmds/cloud/aws_cmd.rs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
66
use crate::core::tee::force_tee_hint;
77
use crate::core::tracking;
8+
use crate::core::truncate::{CAP_INVENTORY, CAP_LIST};
89
use crate::core::utils::{
910
exit_code_from_output, exit_code_from_status, human_bytes, join_with_overflow,
1011
resolved_command, shorten_arn, truncate_iso_date,
@@ -15,7 +16,7 @@ use lazy_static::lazy_static;
1516
use regex::Regex;
1617
use serde_json::Value;
1718

18-
const MAX_ITEMS: usize = 20;
19+
const MAX_ITEMS: usize = CAP_LIST;
1920
const JSON_COMPRESS_DEPTH: usize = 4;
2021

2122
/// Result of a filter function: filtered text + whether items were truncated.
@@ -494,7 +495,7 @@ fn filter_s3_ls(output: &str) -> FilterResult {
494495

495496
if total > limit {
496497
let text = format!(
497-
"{}\n... +{} more items",
498+
"{}\n +{} more items",
498499
lines[..limit].join("\n"),
499500
total - limit
500501
);
@@ -553,7 +554,7 @@ fn filter_ec2_instances(json_str: &str) -> Option<FilterResult> {
553554
}
554555

555556
if truncated {
556-
result.push_str(&format!(" ... +{} more\n", total - MAX_ITEMS));
557+
result.push_str(&format!(" +{} more\n", total - MAX_ITEMS));
557558
}
558559

559560
let text = result.trim_end().to_string();
@@ -700,7 +701,7 @@ fn filter_cfn_describe_stacks(json_str: &str) -> Option<FilterResult> {
700701

701702
// --- P0 filters: CloudWatch Logs, CloudFormation Events, Lambda ---
702703

703-
const MAX_LOG_EVENTS: usize = 50;
704+
const MAX_LOG_EVENTS: usize = CAP_INVENTORY;
704705

705706
/// Convert days since Unix epoch to (year, month, day). Civil calendar, UTC.
706707
fn days_to_ymd(days: i64) -> (i64, i64, i64) {
@@ -759,7 +760,7 @@ fn filter_logs_events(json_str: &str) -> Option<FilterResult> {
759760
}
760761

761762
if truncated {
762-
lines.push(format!("... +{} more events", total - MAX_LOG_EVENTS));
763+
lines.push(format!(" +{} more events", total - MAX_LOG_EVENTS));
763764
}
764765

765766
let text = lines.join("\n");
@@ -1132,7 +1133,7 @@ fn filter_dynamodb_items(json_str: &str) -> Option<FilterResult> {
11321133
}
11331134

11341135
if truncated {
1135-
lines.push(format!("... +{} more items", total - MAX_ITEMS));
1136+
lines.push(format!(" +{} more items", total - MAX_ITEMS));
11361137
}
11371138

11381139
let text = lines.join("\n");
@@ -1426,7 +1427,7 @@ fn filter_logs_query_results(json_str: &str) -> Option<FilterResult> {
14261427
}
14271428

14281429
if truncated {
1429-
lines.push(format!("... +{} more rows", total - MAX_ITEMS));
1430+
lines.push(format!(" +{} more rows", total - MAX_ITEMS));
14301431
}
14311432

14321433
let text = lines.join("\n");
@@ -1616,7 +1617,7 @@ mod tests {
16161617
}
16171618
let input = lines.join("\n");
16181619
let result = filter_s3_ls(&input);
1619-
assert!(result.text.contains("... +20 more items"));
1620+
assert!(result.text.contains(" +20 more items"));
16201621
assert!(result.truncated);
16211622
}
16221623

@@ -1852,7 +1853,7 @@ mod tests {
18521853
}
18531854
let json = format!(r#"{{"DBInstances": [{}]}}"#, dbs.join(","));
18541855
let result = filter_rds_instances(&json).unwrap();
1855-
assert!(result.text.contains("... +5 more instances"));
1856+
assert!(result.text.contains(" +5 more instances"));
18561857
assert!(result.truncated);
18571858
}
18581859

@@ -1893,7 +1894,7 @@ mod tests {
18931894
}
18941895
let json = format!(r#"{{"events": [{}]}}"#, events.join(","));
18951896
let result = filter_logs_events(&json).unwrap();
1896-
assert!(result.text.contains("... +10 more events"));
1897+
assert!(result.text.contains(" +10 more events"));
18971898
assert!(result.truncated);
18981899
}
18991900

0 commit comments

Comments
 (0)