feat(runner): add flashduty_exec operation for capability-bound auth#50
Open
ysyneu wants to merge 4 commits into
Open
feat(runner): add flashduty_exec operation for capability-bound auth#50ysyneu wants to merge 4 commits into
ysyneu wants to merge 4 commits into
Conversation
…ction Adds executeFlashdutyExec that fork-execs the flashduty CLI with per-call auth env, isolated from the generic bash code path. Routes TaskOpFlashdutyExec through the WebSocket handler. Phase 2 of fc-safari CLI adoption.
Phase 2 contract is that only the `flashduty_exec` path carries per-user Flashduty credentials (FLASHDUTY_APP_KEY, FLASHDUTY_API_BASE, ...). The generic `bash` tool must not see them. The previous executeBashCommand inherited the runner's full os.Environ(), which leaked any FLASHDUTY_* keys present in the runner process — easy to hit on a dev workstation that exports them for safari, and impossible to audit in cloud sandboxes when sandbox-manager forwards arbitrary entries. Fix: scrubFlashdutySecrets() drops every FLASHDUTY_* entry from the inherited env before bash sees it. Caller-supplied extraEnv layers on top, so explicit hand-offs still work (test fixtures + non-secret overrides unchanged). The flashduty_exec path is untouched — it relies on safari's per-call extraEnv to re-add FLASHDUTY_APP_KEY in-place. Caught in Phase 2 E2E: bash subprocess reported FLASHDUTY_APP_KEY=<key> even though Phase 2 was supposed to remove it. Re-verified with the scrub: bash sees zero FLASHDUTY_* entries; flashduty tool still returns real incident data via its own auth pathway.
Two CI issues caught after the flashduty_exec PR push: 1. gofmt complained about the FlashdutyExecArgs struct alignment in protocol/messages.go — comment-column re-flow after the new fields. 2. windows-latest unit-test job failed because TestExecuteFlashdutyExec_ VerbSplitsOnWhitespace shells out to /bin/echo to inspect argv. There is no PowerShell equivalent that preserves the same stdout shape, so the test is now skipped on GOOS=windows (mirrors the envd POSIX-only skips landed earlier).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new runner WebSocket operation
flashduty_execthat fork-execs theflashdutyCLI with per-call auth env, separate from the genericbashcode path. This is the runner-side counterpart to fc-safari'sFlashdutyExec(verb, args)(PR flashcatcloud/fc-safari#74).protocol/messages.go:TaskOpFlashdutyExec+FlashdutyExecArgs+FlashdutyExecResult = BashResult(alias keeps safari's existing decoder reusable)environment/environment.go:executeFlashdutyExecmirrorsexecuteBashCommandshape (timeout, env merge, output capture, truncation,.outputs/spill) — only differences are directexec.CommandContext("flashduty", verbTokens..., args...)instead ofbash -c, and$FLASHDUTY_BINARYenv hook for testability. Factored a sharedrunCapturedCommandhelper fromexecuteBashCommandto avoid duplication.ws/handler.go: routesTaskOpFlashdutyExecthroughEnvironment.FlashdutyExec.Why this exists
fc-safari currently injects
FLASHDUTY_APP_KEYinto every generic bash invocation (see PR #74 commitf1d4a06a). That env is now removed from the bash path — audited callers route through this new op instead, so the LLM can no longer leak the key viaenv,echo $FLASHDUTY_APP_KEY, or any accidental echo in skill output.Test plan
go test ./environment -count=1—TestExecuteFlashdutyExec_EnvIsolatedToSubprocess(env confinement) +TestExecuteFlashdutyExec_VerbSplitsOnWhitespace(verb tokenization via/bin/echosubstitution)go test ./... -count=1greengo build ./... && go vet ./...cleanflashduty(...), (b)env | grep FLASHDUTYshows no valuesWire shape (for future safari callers)
{"verb":"incident list","args":["--json","--limit","10"],"workdir":"…","timeout":120,"env":{"FLASHDUTY_APP_KEY":"…","FLASHDUTY_API_BASE":"…"}}Result is
BashResult-shaped (exit_code, stdout/stderr with truncation + spill).