Skip to content

fix(ai-sre): refuse remediation patches outside the watched GitOps layout (JDWLABS-460) - #210

Merged
jdwillmsen merged 2 commits into
mainfrom
fix/JDWLABS-460-remediation-path-guardrails
Aug 29, 2026
Merged

fix(ai-sre): refuse remediation patches outside the watched GitOps layout (JDWLABS-460)#210
jdwillmsen merged 2 commits into
mainfrom
fix/JDWLABS-460-remediation-path-guardrails

Conversation

@jdwillmsen

Copy link
Copy Markdown
Member

Root cause

9/9 relay-authored PRs in jdwlabs/platform (#354-360, #372, #373) were unusable. Three defects, all in apps/backend/ai-sre-relay:

  1. Fabricated pathspatch.go patchSystemPrompt sent the model only "propose one single-file GitOps change" plus the Holmes root-cause text. No repo layout, no clone, no map. github.go safeFilePath only blocked traversal, so manifests/…, cluster/…, clusters/…, apps/…, platform/… were accepted and written as new files (contents GET 404 → create).
  2. Unrelated commits pushed to open PRsgithub.go OpenPR step 2 treated a 422 on branch create ("already exists") as an idempotent re-run and went on to PUT. The branch is derived from the Jira key (fix/ai-sre/jdwlabs-NNN); the in-process refire suppression in pipeline.go is lost on every pod restart (32Mi replica), so each re-investigation of the same ticket produced a fresh, different patch and appended it (#357: 8 commits, 8 different invented paths).
  3. Wrong target — consequence of 1: with no tree to consult the model guessed a resource/file from prose alone.

Fix

  • GITHUB_PATH_ALLOWLIST (path.Match globs) — fail-closed like GITHUB_REPO_ALLOWLIST; refusal is ErrPathNotAllowed, counted as ai_sre_relay_path_rejections_total.
  • The file must already exist on main (checked before the branch is created; PUT always carries the blob SHA). The arm edits, it never creates.
  • 422 on branch create is now ErrBranchExists: no write, no PR, counted as ai_sre_relay_branches_skipped_total. One PR, one commit.
  • Prompt names the watched layout (tenants/<t>/tenant.yaml, services/<release>/values.yaml, services/<release>/postInstall/<name>.yaml) and says to answer {"confidence":0} rather than guess.

Companion: jdwlabs/platform PR adds the env var to the relay manifest and an orphaned-manifest-check CI job.

Test plan

  • go vet ./... && go test -race ./... green (new github_paths_test.go: every path the bot actually proposed is refused before any API call; nonexistent file refused; existing branch refused with no PUT/PR; happy path PUTs once with SHA; prompt names the layout).
  • nx lint ai-sre-relay green.
  • After release: bump the image pin in platform ai-sre-relay.yaml; without GITHUB_PATH_ALLOWLIST the arm logs a warning and opens no PRs.

Refs JDWLABS-460.

🤖 Generated with Claude Code

https://claude.ai/code/session_016XRddumw4ZNvFqHSd9KSf6

@jdwillmsen
jdwillmsen force-pushed the fix/JDWLABS-460-remediation-path-guardrails branch from 2f840ef to 7e8ac96 Compare August 29, 2026 05:04
…Ops layout

Every remediation PR the relay has opened was unusable: the model, given
only a root-cause narrative and no view of the repository, wrote to
invented top-level directories (manifests/, cluster/, clusters/, apps/)
that no ArgoCD Application reads, and repeat proposals for the same ticket
were pushed onto the existing branch because a 422 on branch create was
tolerated as an idempotent re-run.

The PR arm now edits only files that already exist on the base branch and
match a configured path allowlist (GITHUB_PATH_ALLOWLIST, fail-closed like
the repo allowlist), refuses to write onto a branch that already exists,
and the patch prompt names the watched layout and tells the model to
answer with zero confidence rather than guess. Refusals are counted
separately from GitHub failures so a model that keeps inventing layouts
shows up as a trend.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016XRddumw4ZNvFqHSd9KSf6
@jdwillmsen
jdwillmsen force-pushed the fix/JDWLABS-460-remediation-path-guardrails branch from 7e8ac96 to b9469ea Compare August 29, 2026 05:08
Independent review of this PR and its jdwlabs/platform companion found
three real gaps, all fixed here:

- OpenPR treated any 422 on branch create as "a PR for this ticket is
  already in review" and skipped quietly at Info level. But the per-alert
  context is often already dead by the time OpenPR runs (pipeline.go's own
  comment on the Holmes failure path notes this), so a deadline or a
  GitHub 5xx on the PUT or the pulls create can strand a branch with a
  real, unreviewed commit and no PR at all -- and every retry after that
  hit the same 422 and was silently dropped with a false "already in
  review" claim. OpenPR now calls GET .../pulls?state=all on a 422 before
  deciding: a PR in any state is the existing ErrBranchExists skip: an
  empty result is the new ErrBranchOrphaned, logged at Error and counted
  separately (branchesOrphaned) from the expected branchesSkipped.

- GITHUB_PATH_ALLOWLIST's wildcard globs are necessarily coarser than
  "actually reconciled": tenants/*/services/*/values.yaml also matches a
  dormant release's values.yaml, which exists on main and passes every
  gate but renders no Application. GitHubClient now also takes
  deniedPaths, checked before and winning over the allowlist; a match is
  ErrPathDenied, folded into the same pathsRejected counter as
  ErrPathNotAllowed since both mean the same thing to an operator. The
  platform companion PR populates GITHUB_PATH_DENYLIST with the three
  known dormant releases and adds a CI assertion that keeps the two lists
  in step.

- tenants/<tenant>/tenant.yaml is no longer in the prompt's list of
  editable files: it is the tenant's whole service list, rendered by
  governance-appset with prune and self-heal, and no remediation use case
  needs a full-file rewrite of it. The boundary already refused it (it was
  never in GITHUB_PATH_ALLOWLIST), so this only stops the model from
  wasting an attempt proposing it, with an explicit blast-radius note in
  the prompt.

Also fills two test gaps the review flagged: pipeline_test.go now covers
the pathsRejected/branchesSkipped/branchesOrphaned counter and log-level
mappings individually (previously only reposRejected had a test), and
github_paths_test.go adds the failed-PR-creation-after-a-fresh-commit
scenario that actually produces an orphaned branch, alongside the
orphaned-branch detection itself.

Refs JDWLABS-460.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GzFwqYVWFmpgh5h1zNxuWQ
@jdwillmsen
jdwillmsen merged commit 708dcc9 into main Aug 29, 2026
22 checks passed
@jdwillmsen
jdwillmsen deleted the fix/JDWLABS-460-remediation-path-guardrails branch August 29, 2026 20:55
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.

1 participant