Skip to content

Commit 80d5903

Browse files
fix(#6788): address review feedback on dummy-playback runtime
- Validate playback-comment-url path starts with "/" to prevent argument injection via gh/glab CLI flags (both two-line and legacy single-line formats) - Add dummy-playback row to docs/runtimes.md runtime summary table - Add Dummy-playback runtime operations section to docs/contributing/runtime-implementation.md with security matrix note, playlist format, companion files, fix/feature entries, tracking comment, and config key table - Update ValidRuntimes() comment to mention dummy-playback alongside dummy - Group top-level constants into a const() block - Move dummy-playback case adjacent to dummy in registry.go - Add 3 test cases for argument injection prevention Addresses review feedback on #6789
1 parent 5b6757b commit 80d5903

6 files changed

Lines changed: 92 additions & 7 deletions

File tree

docs/contributing/runtime-implementation.md

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,61 @@ The `dummy` runtime executes a YAML script of operations inside the real sandbox
361361
| `assert_file` | `path` | Assert file exists and is readable under the workspace |
362362
| `assert_json` | `path,json_path` | Assert JSON file exists and dot-path field is present and non-null (uses `jq`) |
363363

364+
## Dummy-playback runtime operations
365+
366+
The `dummy-playback` runtime replays canned agent results from an ordered
367+
playlist, enabling fast, deterministic e2e tests that exercise the full
368+
dispatch→pre-script→runtime→post-script pipeline using pre-recorded output.
369+
370+
**Security feature matrix:** N/A — test-only runtime. Like `dummy`, it
371+
does not implement `SandboxHooksBootstrap` and installs no sandbox tool hooks.
372+
No inference calls are made.
373+
374+
### Playlist format
375+
376+
The playlist is a YAML file at `.fullsend/results/playlist.yaml`:
377+
378+
```yaml
379+
current: 1
380+
results:
381+
- triage/sufficient
382+
- review/approve
383+
- code/implemented
384+
```
385+
386+
`current` is 1-indexed. Each entry names a subdirectory under
387+
`.fullsend/results/` that contains a `result.json` and optional companion
388+
files. The runtime serves the entry at `current`, writes `result.json` to
389+
the sandbox output directory, copies companion files into the workspace,
390+
and advances `current` via a local write (or git commit+push in
391+
production).
392+
393+
### Companion files
394+
395+
Files in the entry directory (other than `result.json`) are uploaded to the
396+
sandbox. Files under a `repo/` subdirectory are placed relative to the
397+
target repo checkout (simulating code changes); all other files go to the
398+
workspace root.
399+
400+
### Fix vs. feature entries
401+
402+
Entries whose name starts with `fix/` commit to the current branch
403+
(preserving the PR branch for post-fix push). All other entries create a
404+
new feature branch (`fullsend/playback-<entry>`).
405+
406+
### Tracking comment
407+
408+
An optional `playback-comment-url` file in the `.fullsend/` directory
409+
enables cross-iteration playlist tracking via a forge API comment. The
410+
file format is `<cli>\n<path>` (e.g. `gh\n/repos/…/comments/42` or
411+
`glab\n/projects/…/notes/99`). Legacy single-line files default to `gh`.
412+
413+
### Config key table
414+
415+
`dummy-playback` does not support any harness config keys — it replays
416+
pre-recorded results and ignores `model`, `effort`, `tools`, and other
417+
runtime-neutral keys.
418+
364419
## pi runtime internals (#6464)
365420

366421
User-facing pi behaviour is in [Pi](../runtimes/pi.md). This section keeps the

docs/runtimes.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ sandbox, the credentials, and the verdict.
99
| **[`claude`](runtimes/claude.md)** | Production agent runs (Claude Code) | Default |
1010
| **[`pi`](runtimes/pi.md)** | Second runtime, opt-in per repo — Claude, Grok and Gemini on Vertex; GPT via OpenAI WIF (wired, not yet exercised live) | Supported for `triage`, `prioritize`, `code`, `fix` |
1111
| `dummy` | Behaviour tests — scripted ops, no inference | Internal |
12+
| `dummy-playback` | Behaviour tests — replays canned agent results from a playlist, no inference | Internal |
1213
| `opencode` | Not yet functional | Stub |
1314

1415
Pick one with `runtime:` in `.fullsend/config.yaml`, or per run with `--runtime`.

internal/config/config.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,8 @@ func ValidProviders() []string {
297297
}
298298

299299
// ValidRuntimes returns the set of recognized agent runtimes. "pi" is
300-
// opt-in per org/repo (#6464); "dummy" is for behaviour test orgs only.
300+
// opt-in per org/repo (#6464); "dummy" and "dummy-playback" are for
301+
// behaviour test orgs only.
301302
func ValidRuntimes() []string {
302303
return []string{"claude", "pi", "dummy", "dummy-playback"}
303304
}

internal/runtime/dummy_playback.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ import (
1919
"github.com/fullsend-ai/fullsend/internal/ui"
2020
)
2121

22-
const playlistRelPath = "results/playlist.yaml"
23-
const playbackCommentFile = "playback-comment-url"
24-
const resultFileName = "result.json"
22+
const (
23+
playlistRelPath = "results/playlist.yaml"
24+
playbackCommentFile = "playback-comment-url"
25+
resultFileName = "result.json"
26+
)
2527

2628
// Playlist is the YAML committed to .fullsend/results/playlist.yaml.
2729
// Results are 1-indexed: current=1 serves results[0].
@@ -322,6 +324,12 @@ func parsePlaybackCommentRef(data string) (playbackCommentRef, bool) {
322324
return playbackCommentRef{}, false
323325
}
324326
if !found {
327+
// Legacy single-line format: the entire value is the API path.
328+
// Validate it starts with "/" to prevent argument injection
329+
// (e.g. "--hostname=attacker.com" interpreted as a flag by gh).
330+
if !strings.HasPrefix(cli, "/") {
331+
return playbackCommentRef{}, false
332+
}
325333
return playbackCommentRef{cli: "gh", path: cli, method: "PATCH"}, true
326334
}
327335
if cli != "gh" && cli != "glab" {
@@ -331,6 +339,10 @@ func parsePlaybackCommentRef(data string) (playbackCommentRef, bool) {
331339
if path == "" {
332340
return playbackCommentRef{}, false
333341
}
342+
// Validate path starts with "/" to prevent argument injection.
343+
if !strings.HasPrefix(path, "/") {
344+
return playbackCommentRef{}, false
345+
}
334346
method := "PATCH"
335347
if cli == "glab" {
336348
method = "PUT"

internal/runtime/dummy_playback_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,21 @@ func TestParsePlaybackCommentRef(t *testing.T) {
646646
input: "bash\n-c whoami",
647647
wantOK: false,
648648
},
649+
{
650+
name: "argument injection in legacy single-line",
651+
input: "--hostname=attacker.com",
652+
wantOK: false,
653+
},
654+
{
655+
name: "argument injection in two-line path",
656+
input: "gh\n--hostname=attacker.com",
657+
wantOK: false,
658+
},
659+
{
660+
name: "relative path rejected in two-line",
661+
input: "gh\nrepos/org/repo/issues/comments/42",
662+
wantOK: false,
663+
},
649664
}
650665

651666
for _, tt := range tests {

internal/runtime/registry.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,16 @@ func Resolve(name string) (Backend, error) {
1818
// Selected only via explicit per-repo/org config (behaviour test orgs).
1919
r := DummyRuntime{}
2020
return Backend{Runtime: r, Transcripts: r}, nil
21+
case "dummy-playback":
22+
// Replays canned results from a playlist (behaviour test orgs).
23+
r := DummyPlaybackRuntime{}
24+
return Backend{Runtime: r, Transcripts: r}, nil
2125
case "opencode":
2226
r := OpenCodeRuntime{}
2327
return Backend{Runtime: r, Transcripts: r}, nil
2428
case "pi":
2529
r := PiRuntime{}
2630
return Backend{Runtime: r, Transcripts: r}, nil
27-
case "dummy-playback":
28-
r := DummyPlaybackRuntime{}
29-
return Backend{Runtime: r, Transcripts: r}, nil
3031
default:
3132
return Backend{}, fmt.Errorf("unknown runtime %q: must be one of %s", name, strings.Join(config.ValidRuntimes(), ", "))
3233
}

0 commit comments

Comments
 (0)