repository-routing-security

Imported from kunchenguid/no-mistakes (.agents/skills/repository-routing-security/SKILL.md). Install upstream with npx skills add kunchenguid/no-mistakes --skill repository-routing-security. Copyright stays with the author.

Fork Routing

• repos.upstream_url is the parent repository used for PR base routing; repos.fork_url is an optional GitHub fork push target.
• no-mistakes init --fork-url <url> expects origin to point at the GitHub parent repository and <url> at the contributor fork; plain no-mistakes init preserves an existing fork URL on idempotent refresh.
• Push code must resolve the push URL via resolvePushURL (internal/pipeline/steps/common_git.go) so configured forks still receive branch updates, including after a CI repair restarts validation; the non-fork path recovers the credentialled upstream from the worktree's origin remote at run time because the DB upstream_url is stored redacted (see Credential Redaction below). Repo.PushURL() remains correct only for fork-only callers (e.g. rebase.go), since fork URLs carry no embedded credentials.
• GitHub PR code must keep --repo pointed at the parent and use --head <fork_owner>:<branch> when fork_url is set; existing-PR lookup must list by the bare branch and filter head-owner fields, never pass <owner>:<branch> to gh pr list --head.
• Non-GitHub fork MR/PR routing is intentionally out of scope until implemented end to end; if a legacy row has fork_url for another provider, PR creation must skip instead of opening a self PR.
• Every new run best-effort refreshes registered upstream/fork URLs from the working clone through gate.RefreshRepoURLs: origin is the upstream authority, an existing fork requires one uniquely matching clone remote, both DB fields replace atomically, and every discovery/validation/write failure logs only a bounded reason and continues with the exact old registration. The refresh never rewrites clone or gate remotes; Repo.URLsVerified is run-scoped evidence that trusted fetch/push may use the refreshed DB URL instead of an inherited stale gate origin.
• Fork-as-origin detection (internal/gate/forklayout.go, issue #1178): a plain init (no --fork-url, no previously recorded fork) refuses when origin is a GitHub fork and a separate upstream remote already names its real parent - the layout gh repo fork --clone leaves behind, which otherwise opens scratch PRs inside the contributor's own fork and never binds a parent-PR attestation. Contract is restore-style (VISION R1: refuse loudly with --fork-url/CONTRIBUTING guidance), not auto-adopt, per the maintainer's triage on #1178 (opt-in was the acceptable alternative; silently rewriting remotes or always auto-rerouting was explicitly rejected). Detection fails OPEN, never closed: it needs positive proof (github.RepoParent's gh api repos/<slug> "fork" + "parent" answer for origin matching what upstream names) and skips silently otherwise - no upstream remote, either remote not on GitHub, an unrelated upstream (different repo name, or a different host from origin - both checked locally before ever shelling out to gh, so init never pays network latency for a template/reference upstream), or gh unavailable/unauthenticated/offline. The host check exists because a GitHub fork always shares its parent's host (forking across GitHub.com/GHE instances does not exist), so an upstream on a different host can never be origin's parent however its owner/name happen to read - comparing owner/name alone would wrongly match two unrelated same-named repos on different hosts (Greptile finding on PR #1183). resolveForkParent (package var in internal/gate/forklayout.go) is the test seam over the real gh call, mirroring ensureGateHooksPathIsolation.

Repository Forge Identity (internal/forgecontext)

• Optional global forge_profiles map raw remote host tokens/SSH aliases to one isolated gh or glab config directory, plus an optional expected_login pin. The resolver owns profile selection, validation, parent/fork ambiguity, provider-specific fail-closed activation, and the immutable run environment; do not add ambient account switching or per-step routing. Profile identity for the parent/fork same-profile check is the config directory AND the pin, so conflicting pins fail as ambiguous instead of silently picking one account (sameProfile/expectLogin own the rationale).
• A resolved context must reach built-in provider commands, configured shell commands, native agents, managed agent servers, and recovered approval reconciliation. Never mutate the daemon environment or persist credentials/profile selection in the DB; recovery re-resolves from current global config.
• No configured profiles means exact legacy ambient behavior. Online auth failures keep provider steps' existing skip behavior; deterministic config/routing errors fail before the pipeline. The public contract lives in docs/src/content/docs/reference/global-config.md.
• Global provider_plugins (internal/config/provider_plugins.go) claim hosts for an operator-installed executable (internal/scm/plugin, contract in reference/provider-plugin-protocol.md). Merge takes them from global config only; never let repository config add, select, or reconfigure one. buildPluginHost (internal/pipeline/steps/host.go) selects a plugin before forge profiles or built-in detection, sends only the redacted remote URL (as the --remote-url context flag), and skips with a reason (never self-PRs) when the repo has a fork_url, exactly like the non-GitHub built-ins, so the skip surfaces under run.automatic_skips. A host claimed by both a plugin and a forge profile is rejected at config load, and forgecontext.RefuseProviderPluginOverlap (called right after forgecontext.Resolve at run start and recovery in internal/daemon/manager.go) refuses the overlap config load cannot see: a profile keyed by an SSH alias while a plugin claims its resolved HostName. Every plugin-returned PR URL must contain the --repo path as whole segments and, once the run holds a URL, equal it (samePR); the host is deliberately not compared because SSM-style forges serve the web UI from a different host than git. Regressions: TestMergeTakesProviderPluginsFromGlobalOnly, TestLoadGlobalRejectsHostClaimedByForgeProfileAndPlugin, TestRefuseProviderPluginOverlap, internal/pipeline/steps/provider_plugin_test.go.

Credential Redaction in Stored URLs and Errors (security)

• gate.InitWithFork runs the upstream URL through safeurl.Redact before every DB persist (UpdateRepoMetadata*, InsertRepoWithIDAndFork) and the "gate initialized" log line; the bare gate's origin remote still carries the full credentialled URL (via provisionGate) so carved worktrees authenticate. Because the DB copy is redacted, push and branch-sync code must recover the credential from the worktree's origin remote at run time (resolvePushURL/resolveUpstreamURL), never from Repo.UpstreamURL/Repo.PushURL().
• Step-failure errors (executor.go FailStep/log/IPC emit) and the Bitbucket resolve-repo error are redacted via safeurl.RedactText/safeurl.Redact so a credentialled URL wrapped into an error can never reach a step log or runs.error. Reuse internal/safeurl for new redaction sites rather than adding a git-local helper; it is already wired into git.Run/step git-run error formatting.
• Regressions: TestInitRedactsCredentialURL, TestResolveUpstreamURL_PreservesCredential, TestResolveUpstreamURL_FallsBackToRecordedURL, TestResolvePushURL_ForkWinsOverCredential.

Repo Config Trust Boundary (security)

• The daemon runs commands.* from .no-mistakes.yaml verbatim via sh -c, and agent selects which process launches with the maintainer's credentials. The code-executing selection fields (commands.{prepare,test,lint,format} and agent) are therefore loaded from the trusted default branch at a pinned SHA resolved by a fresh fetch, never from the pushed SHA. The run aborts when the trusted commit or its present config cannot be read and parsed; a readable tree with no config is valid. See internal/daemon/manager.go startRun, loadTrustedRepoConfig, and assertGateTrustedConfigReadable.
• document.instructions (the repo's documentation placement policy), review.path_instructions (path-scoped review guidance appended to the review prompt), review.agent_timeout (a repository can only tighten the global review_agent_timeout; the pushed branch it gates cannot set it), gates (extra repository-declared checks), auto_fix.gates (their opt-in fix budgets), disable_project_settings (the gate-agent project-instruction opt-out), no_ci (positive declaration that the repository intentionally has no CI), the whole ci block, pr.template (path and pinned bytes), pr.publish_intent, pr.appendix, test.prepare (the eager setup trigger for agent-only Test), test.base_attribution (re-running a failing commands.test on the base commit), test.instructions (see the Live-Validation Contract in the pipeline-review-and-agents skill), and test.allow_approve_over_failure (the recorded reason that opts require-no-mistakes into accepting a Test step approved over a failing commands.test) are also trusted-only, regardless of allow_repo_commands: a pushed branch must not weaken any of those boundaries, self-declare no-CI to bypass checks, steer its own review or its own live validation, enable eager setup or a base-commit suite run, raise the maintainer-funded ci.rerun_transient budget, turn off required post-repair revalidation, or waive the required check for an approved-over-failure test command. The operator's global CI settings are a separate, non-contributor surface that trusted repo values override. Enabling the commands opt-in must not drop the maintainer's own trusted values. When the project-settings opt-out is enabled, only adapters with verified effective suppression may launch. Other non-executing fields (ignore_patterns, core-step auto_fix, commit, intent, and the remaining test fields) are still read from the pushed branch.
• Operator-owned review/document/test guidance (issue #1019) is an allowlist: global review.path_instructions (OperatorReviewRaw, no conversation) and repository_overrides.<remote>.{review.path_instructions,document.instructions,test.instructions}. Each renders as its own labeled source before the trusted rules (config.Review.PathInstructionSources) and never replaces them; do not widen it to fields that can weaken a gate. The 32-entry/16,384-byte caps bound the COMBINED set: the global config validates global+each override at load, and Review.ValidatePathInstructionsBudget re-checks with the trusted copy at run start and recovery, failing closed. Regressions: internal/config/operator_instructions_test.go, TestStartRun_FailsClosedWhenOperatorAndTrustedPathInstructionsOverrunTheCap, TestLoadRecoveredConfig_FailsClosedWhenOperatorAndTrustedPathInstructionsOverrunTheCap, TestReviewStep_OperatorPathInstructionsRenderAsLabeledSectionsBeforeTrusted, TestReviewPathInstructionSourcesStayWithinAccountedBytes, TestTestStep_MachineLocalRunbookPrecedesTrustedAndPushedStaysOut.
• Selecting which trusted config applies to a run must never depend on a pushed-branch field. review.path_instructions is matched against the COMPLETE changed-file set, never the ignore_patterns-filtered subset, because filtering there lets a contributor suppress a maintainer's rule from their own review by ignoring its glob. reviewPathsAndInstructions (internal/pipeline/steps/review_path_instructions.go) owns the final review scope and rendered guidance before the empty-scope shortcut; reviewablePaths (common_diff.go) only filters paths by a supplied ignore list. EffectiveRepoConfig preserves the trusted ignore list separately as TrustedIgnorePatterns. User-facing scope semantics are owned by docs/src/content/docs/reference/repo-config.md (review.path_instructions). Regressions: TestReviewStep_IgnorePatternsCannotWaiveTrustedInstructions, TestEffectiveRepoConfig_TrustedReviewIgnores, TestReviewPathsAndInstructions_SettlesEverySourceBeforeIgnoring.
• pr.base_branch (integration and change-scoping semantics: docs/src/content/docs/reference/pipeline-steps.md) is trusted-default-branch-only, but unlike the fields in the bullet above it is the deliberate exception that also honors the allow_repo_commands: true opt-in, since it controls where an already-maintainer-authorized PR lands rather than what executes. Once a PR already exists, its actual forge base branch (read live via scm.PRBaseBranchReader) is authoritative for CI merge-conflict repair and base-branch tip monitoring over a since-changed pr.base_branch, and PR lookup matches the existing PR by branch alone, never filtered by base, so a later config change updates that PR instead of opening a duplicate against the new base. Full semantics are owned by docs/src/content/docs/reference/repo-config.md (pr.base_branch). Regressions: TestEffectiveRepoConfig_PRBaseBranchTrustedOnly, TestEffectiveRepoConfig_PRBaseBranchOptInUsesPushedValue, TestEffectiveRepoConfig_PRBaseBranchOptInWithNoTrustedCopyUsesPushedValue, TestLoadRepoConfig_PRBaseBranchRejectsInvalidBranchName, TestLoadRepoConfig_PRBaseBranchEmptyIsValid, TestPRStep_UsesConfiguredBaseBranch, TestRebaseStep_UsesConfiguredPRBaseBranch, TestCIStep_AutoFixUsesExistingPRBaseAfterConfigChanges, TestPRStep_ExistingPRAgainstDifferentBaseIsUpdatedNotDuplicated.
• allow_repo_commands is per-repo, read only from the trusted default-branch copy, and defaults false; a contributor cannot self-enable it from a pushed branch. The e2e harness models a trusted single-developer environment and commits allow_repo_commands: true via SetupOpts.AllowRepoCommands; security tests pass false.
• Regressions: TestLoadTrustedRepoConfig_FailClosedOnFetchFailure, TestLoadTrustedRepoConfig_PinnedSHAReadsFreshDefaultBranch, TestEffectiveRepoConfig_DocumentPolicyTrustedOnly, TestEffectiveRepoConfig_ReviewPathInstructionsTrustedOnly, TestMatchPathInstructions_PushedIgnorePatternsCannotSuppressTrustedRule, TestReviewStep_PushedIgnorePatternsCannotSuppressPathInstructions, TestEffectiveRepoConfig_DisableProjectSettingsTrustedOnly, TestEffectiveRepoConfig_CIRerunTransientTrustedOnly, TestEffectiveRepoConfig_AllowApproveOverFailureTrustedOnly, TestTestPrepareTrustAndMerge, TestTestBaseAttributionTrustAndMerge, TestAssertGateTrustedConfigReadable_*, TestNewPipelineAgent_OptOut_*, TestLoadRecoveredConfig_BoundsFetchAndFailsClosed, e2e TestRepoConfigCommandsFromDefaultBranch (incl. pushed_branch_cannot_self_enable), e2e TestReviewPathInstructionsJourney.

Gate-Agent Instruction Neutralization (disable_project_settings)

• Under the trusted opt-out, agent.EnsureGateNeutralized refuses any gate agent that does not neutralize the TARGET repo's project instructions; only adapters whose suppression knob is empirically verified implement NeutralizesGateInstructions() and only while the knob is actually in effect (an operator override that defeats it reports false and fails closed). Verified: codex (project_doc_max_bytes=0 + --ignore-rules), claude (--setting-sources user), pi (--no-context-files), and the acp:omp ACP target. Grok is deliberately refused (it still discovers native project instructions).
• acp:omp neutralization lives in internal/agent/ompgate.go and is applied by the acpxAgent (internal/agent/acpx.go): its default launch is switched to --agent "omp acp --config <overlay> --no-rules --no-skills --no-extensions", where the overlay disables every omp context-file discovery provider AND mnemopi memory. omp has NO CLI flag for context files (--no-rules governs only RULES.md); the --config overlay is the highest settings layer, so a hostile repo .omp/config.yml cannot re-enable a disabled provider. Memory is disabled because a gate turn that reads the repo's AGENTS.md can otherwise auto-retain it and a later turn recall it around the provider suppression (a cross-turn vector omp has and pi does not). Only the default launch qualifies: an acp_registry_overrides entry for omp is an opaque custom command and fails closed, as does any non-omp acp:<target>. Empirically verified against omp 18.2.0 through the real acpx->omp path.
• Regressions: internal/agent/gateneutralize_test.go (TestNeutralizesGateInstructions_OMPTargetUnderOptOut, TestOMPGateNeutralization_AppliesSuppressionOverlayAndFlags), internal/daemon/gateneutralize_test.go (TestNewPipelineAgent_OptOut_AdmitsOMPACPTarget). User semantics: docs/src/content/docs/reference/repo-config.md (disable_project_settings).

Repository-Declared Extra Gates (gates)

• steps.WithCustomGates inserts each configured gate immediately after its core anchor. Gates can never skip, reorder, or replace a core step. Valid anchors are rebase, review, test, document, and lint; intent and the delivery tail are refused. User-facing semantics live in docs/src/content/docs/reference/repo-config.md.
• A gate step name is gate.<anchor>.<label>. types.ValidCustomGateLabel owns the path-safe label syntax. GetStepsByRun and the attestation use deterministic tie-breaks because gates share their anchor's step order.
• The daemon decides a run's gate list once, stores it in runs.gates_json, and reads the pin during recovery. An absent pin means the core pipeline. An unparseable pin fails recovery closed.
• validReadableStep accepts gate names for read-only logs. validStep still controls run mutations, so --skip and no-mistakes.skip= refuse gate names.
• Regressions: internal/config/gates_test.go, internal/types/gates_test.go, internal/pipeline/steps/customgate_test.go, internal/daemon/gate_pin_test.go, TestGetStepsByRun_PlacesAGateAfterItsAnchor, TestBuildPipelineAttestation_ListsAGateAfterItsAnchor, TestAxiLogsRefusesAGateStepNameThatWouldEscapeTheLogDirectory, TestSkipPushOptionRefusesARepositoryGateStepName.