fix(review): enforce reviewer read-only in default mode and pass review body via stdin

Address review 4577706991.

- Launch the claude-code reviewer in PermissionModeDefault instead of
  PermissionModeAuto. Auto mode auto-approves unlisted, classifier-"safe"
  tools, so a filesystem-mutating Bash command (> file, sed -i, rm) fell
  through the allow/deny lists. Default mode honors the allowlist and stalls
  every other tool (no human on the headless pane), so read-only is enforced
  rather than classifier-dependent. Reconcile the comment, which described
  default-mode semantics while the code set auto.
- Replace the shell-fragile `--body-text "<markdown>"` prompt step with a
  quoted heredoc piped to `ao review submit --body -`. Review Markdown
  routinely contains ", `, $, and \, which break a double-quoted arg or
  trigger expansion; stdin passes them through literally and writes no file.
- Add `--body -` stdin support to the review submit command, with a test that
  round-trips Markdown containing those shell-breaking characters.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: neversettle17-101

backend/internal/adapters/reviewer/claudecode/claudecode.go

@@ -33,12 +33,16 @@ var _ ports.Reviewer = (*Reviewer)(nil)
 
 // reviewerAllowedTools is the read-only tool allowlist the reviewer launches
 // with. The reviewer runs headless (no human to approve prompts) but must stay
-// read-only, so instead of bypassPermissions — which skips the permission
-// system entirely and ignores allow/deny rules — it launches in the default
-// mode where these rules are honored: allow rules auto-approve without
-// prompting, so the reviewer can read the checkout and run the few commands it
-// needs (git diff/log/show to inspect the PR, gh to post the review, and
-// `ao review submit` to record the verdict) without stalling.
+// read-only. It launches in default mode (not auto, and not bypassPermissions —
+// which skips the permission system entirely and ignores allow/deny rules). In
+// default mode the allow rules auto-approve without prompting, so the reviewer
+// can read the checkout and run the few commands it needs (git diff/log/show to
+// inspect the PR, gh to post the review, and `ao review submit` to record the
+// verdict) without stalling, while any tool that is neither allowed nor denied —
+// including a filesystem-mutating Bash command — prompts and, with no human on
+// the pane, stalls rather than executing. That is what makes read-only an
+// enforced guarantee rather than a classifier-dependent one: auto mode would let
+// a "safe"-classified shell write through, default mode cannot.
 var reviewerAllowedTools = []string{
 	"Read",
 	"Grep",
@@ -70,10 +74,13 @@ func (r *Reviewer) ReviewCommand(ctx context.Context, inv ports.ReviewInvocation
 		WorkspacePath: inv.WorkspacePath,
 		Prompt:        inv.Prompt,
 		SystemPrompt:  inv.SystemPrompt,
-		// Launch off bypassPermissions so the allow/deny lists are enforced.
-		// Set an explicit non-bypass mode instead of deferring to the user's
-		// Claude defaultMode, which may itself be bypassPermissions.
-		Permissions:     ports.PermissionModeAuto,
+		// Launch in default mode, not bypassPermissions (which ignores the
+		// allow/deny lists) and not auto (which would auto-approve an unlisted,
+		// classifier-"safe" shell write). Default mode honors the allowlist and
+		// stalls every other tool, so read-only is enforced rather than left to
+		// the auto classifier. Set it explicitly instead of deferring to the
+		// user's Claude defaultMode, which may itself be bypassPermissions.
+		Permissions:     ports.PermissionModeDefault,
 		AllowedTools:    reviewerAllowedTools,
 		DisallowedTools: reviewerDisallowedTools,
 	})

backend/internal/adapters/reviewer/claudecode/claudecode_test.go

@@ -45,11 +45,13 @@ func TestReviewCommandLaunchesReadOnlyOffBypass(t *testing.T) {
 		t.Fatalf("ReviewCommand: %v", err)
 	}
 
-	// The allowlist is what enforces read-only, so it must launch in an
-	// explicit non-bypass mode: bypassPermissions ignores allow/deny rules
-	// entirely, and an empty mode would defer to a user's defaultMode.
-	if agent.got.Permissions != ports.PermissionModeAuto {
-		t.Fatalf("reviewer must launch in auto permission mode; got %q", agent.got.Permissions)
+	// The allowlist is what enforces read-only, so it must launch in default
+	// mode: bypassPermissions ignores allow/deny rules entirely, auto would
+	// auto-approve unlisted classifier-"safe" shell writes, and an empty mode
+	// would defer to a user's defaultMode. Default mode honors the allowlist and
+	// stalls every other tool.
+	if agent.got.Permissions != ports.PermissionModeDefault {
+		t.Fatalf("reviewer must launch in default permission mode; got %q", agent.got.Permissions)
 	}
 	if !contains(agent.got.AllowedTools, "Read") || !contains(agent.got.AllowedTools, "Bash(ao review submit:*)") {
 		t.Fatalf("allowlist missing read-only review tools: %#v", agent.got.AllowedTools)

backend/internal/cli/review.go

@@ -3,6 +3,7 @@ package cli
 import (
 	"errors"
 	"fmt"
+	"io"
 	"net/url"
 	"os"
 	"strings"
@@ -67,7 +68,7 @@ func newReviewSubmitCommand(ctx *commandContext) *cobra.Command {
 	cmd.Flags().StringVar(&opts.session, "session", "", "Worker session id (or pass it as the positional argument)")
 	cmd.Flags().StringVar(&opts.runID, "run", "", "Review run id (required)")
 	cmd.Flags().StringVar(&opts.verdict, "verdict", "", "Review verdict: approved or changes_requested (required)")
-	cmd.Flags().StringVar(&opts.body, "body", "", "Path to a Markdown file with the review body")
+	cmd.Flags().StringVar(&opts.body, "body", "", "Path to a Markdown file with the review body, or - to read it from stdin")
 	cmd.Flags().StringVar(&opts.bodyText, "body-text", "", "Review body as inline Markdown (use instead of --body when no file should be written)")
 	return cmd
 }
@@ -96,6 +97,12 @@ func (c *commandContext) submitReview(cmd *cobra.Command, args []string, opts re
 	switch {
 	case opts.bodyText != "":
 		body = opts.bodyText
+	case path == "-":
+		raw, err := io.ReadAll(cmd.InOrStdin())
+		if err != nil {
+			return usageError{fmt.Errorf("read body from stdin: %w", err)}
+		}
+		body = string(raw)
 	case path != "":
 		raw, err := os.ReadFile(path)
 		if err != nil {

backend/internal/cli/review_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -81,6 +82,32 @@ func TestReviewSubmitReadsInlineBody(t *testing.T) {
 	}
 }
 
+func TestReviewSubmitReadsBodyFromStdin(t *testing.T) {
+	cfg := setConfigEnv(t)
+	srv, capture := reviewServer(t, http.StatusOK, `{"review":{"verdict":"changes_requested"}}`)
+	writeRunFileFor(t, cfg, srv)
+
+	// Markdown a real review carries: quotes, backticks, $ and a backslash — the
+	// characters that break a double-quoted --body-text arg. Stdin passes them
+	// through untouched.
+	md := "Found a bug in `cmd \"run\"`: $HOME isn't escaped (see C:\\path)."
+	deps := aliveDeps()
+	deps.In = strings.NewReader(md)
+
+	_, errOut, err := executeCLI(t, deps,
+		"review", "submit", "mer-1", "--run", "run-1", "--verdict", "changes_requested", "--body", "-")
+	if err != nil {
+		t.Fatalf("unexpected error: %v\nstderr=%s", err, errOut)
+	}
+	var req submitReviewRequest
+	if err := json.Unmarshal([]byte(capture.body), &req); err != nil {
+		t.Fatalf("decode body: %v", err)
+	}
+	if req.Body != md {
+		t.Fatalf("stdin body not forwarded verbatim; got %q want %q", req.Body, md)
+	}
+}
+
 func TestReviewSubmitBodyAndBodyTextIsUsageError(t *testing.T) {
 	setConfigEnv(t)
 	_, _, err := executeCLI(t, aliveDeps(),

backend/internal/review/prompt.go

@@ -25,9 +25,11 @@ Do these steps in order:
 2. Post your review on the pull request with `+"`gh`"+`, with inline comments for specific findings:
    - If changes are needed, request changes.
    - If it is ready, approve it. GitHub does not let you approve a PR you opened; if the approval is rejected because you are the PR author, post the same review as a regular comment instead (a COMMENT-event review whose body states it is an approval).
-3. Record the result with AO by passing your full review inline:
+3. Record the result with AO. Pass your full review on stdin via a quoted heredoc so quotes, backticks, $, and backslashes in the Markdown are passed through literally and never break the shell or trigger expansion. Keep the closing REVIEW on its own line with no indentation:
 
-    ao review submit --session %s --run %s --verdict <approved|changes_requested> --body-text "<your full review as Markdown>"
+ao review submit --session %s --run %s --verdict <approved|changes_requested> --body - <<'REVIEW'
+<your full review as Markdown>
+REVIEW
 
 Only if step 2 genuinely fails on the provider, still run step 3 so the result is recorded.`,
 		spec.PRURL, spec.TargetSHA, spec.WorkerID, spec.RunID)