[freeqaz]

Problem

Before the Credential Proxy change, native repository Git ran requireConfiguration(GitHubRepoDriver, ...): the JSON schema (sessionDurationSeconds integer bounds, no unknown options) and the path checks (absolute, canonical controlSocket and publicCaPath). Since then, composeRepositoryAccess and githubRepositoryConfiguration call GitHubNativeRepositorySender.validateConfiguration(...) and discard the returned errors, and the native branch never runs the schema. A repository.nativeSender.publicCaPath such as /etc/openclaw/repository-ca/./ca.crt loads without a refusal. (Other schema checks still ran through the repository policy, and the control socket client checks its own path.)

Change

• composition/repository-credentials/platform.ts: run requireConfiguration with the native sender's contract for native Git, as the protected path already does with GitHubRepositoryPolicy.
• drivers/repo/github/installation.ts: drop the discarded validateConfiguration call. Composition validates the same merged object, and the GitHub installation parser stays inside the repository credential import boundary (scripts/verify-repository-credentials-boundary.mjs, unchanged).
• The contract is the one native Git had before: same schema, same path checks. Nothing is loosened.

Verification

• tests/integration/configuration-startup.test.mjs ("repository startup rejects ... CA inputs"): a new case (existing CA file under a non-canonical path) expects ConfigurationValidationError with the exact detail path and code. It fails on main ("Missing expected rejection") and passes here. The whole file passes (35/35); the repository startup tests pinned 20/20 at 2 CPUs.
• Independent internal review: approve with nits. Applied: dropped two cases that the repository policy schema already refused on main (duplicate coverage). Follow-up noted: the refusal detail does not name repository.nativeSender.
• repository-credentials-admission (34/34), repository-native-cutover (7/7), profile-public-github-read (2/2), repository-credentials-package (11/11) and the repository-credentials-contracts conformance file (113/113) pass.
• tsc --build apps/controller passes; scripts/verify-workspace-boundary.mjs and tests/conformance/repository-credentials-source-boundary.test.mjs (142/142) pass; Prettier passes on the touched files. (An earlier head imported requireConfiguration into the installation parser and the boundary check refused it; that import is gone.)

Risks

• An Installation that relied on the missing check (a relative or non-canonical CA path) now refuses at startup, and the Helm upgrade check refuses it before the API restarts. Such an Installation was outside the documented contract.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs changes before merge.

What this changes

This PR enforces the existing native repository sender configuration contract during loading and composition.

Example: An Installation sets repository.nativeSender.publicCaPath to /etc/openclaw/repository-ca/./ca.crt.

• Before: The loader discards validation errors and can read the existing CA file.
• After: The loader rejects the noncanonical path with ConfigurationValidationError.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	A focused, source-supported repair with meaningful loader coverage, one low-impact diagnostic defect, and unverified startup upgrade coverage.
Proof confidence	🦐 gold shrimp (3/6)	This collaborator-authored PR is outside the ordinary live-proof gate; supplied evidence reports real Installation-loader regression coverage, while fresh-install and retained-configuration upgrade startup evidence remains absent.
Patch quality	🐚 platinum hermit (4/6)	1 actionable review finding remain.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Invalid native repository CA paths can survive configuration loading despite the sender's existing path contract.
Reason: The collaborator-authored direction restores established validation through existing composition owners without adding settings or changing Driver contracts.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Needs changes before merge - 1 item remains

Keep this PR open: it repairs a current-main validation regression, with one actionable diagnostic defect and overlapping work to coordinate.

Priority: P2
Reviewed head: 87c38f94fb377a685e510714dac6f8ce32263f15

Before merge

• Identify the invalid native sender path and its correction (P3) - With publicCaPath set to /etc/openclaw/repository-ca/./ca.crt, this newly enforced call throws a ConfigurationValidationError whose detail has path "" and message "Configuration violates the Driver's constraints." The startup formatter preserves that generic detail, so operators cannot identify the failing setting or learn that a canonical absolute path is required. Return a field-specific diagnostic with the path requirements and corrective action, and update the regression assertion accordingly.

Findings

• [P3] Identify the invalid native sender path and its correction — apps/controller/src/drivers/repo/github/installation.ts:41-44

Tests

• Missing end-to-end proof: No fresh-install and retained-configuration upgrade startup run is recorded for the stricter loading path; the reported base-fail/head-pass loader test is meaningful integration coverage.

Agent review details

How this fits together

Installation loading combines repository policy and native transport settings, validates them, and constructs repository policy and optional credential delivery clients.

flowchart TD
  A[Installation configuration] --> B[Repository selection]
  B --> C[Shared configuration validation]
  C --> D[Startup refusal]
  C --> E[Registry and CA loading]
  E --> F[Repository policy and native sender]

Loading

Technical review

Best possible solution:

Land one coordinated repair that uses shared configuration admission and reports the exact invalid native sender field with its required correction.

Do we have a high-confidence way to reproduce the issue?

Current-main source shows discarded path-validation diagnostics followed by CA loading, and the added loader case uses an existing CA file with /./ in its path; the author reports base failure and head success, which this read-only review did not execute.

Is this the best way to solve the issue?

Using requireConfiguration at the existing owners is a small, contract-preserving repair; the resulting startup refusal also needs an actionable field-specific diagnostic.

Full review comments:

• [P3] Identify the invalid native sender path and its correction — apps/controller/src/drivers/repo/github/installation.ts:41-44
  With publicCaPath set to /etc/openclaw/repository-ca/./ca.crt, this newly enforced call throws a ConfigurationValidationError whose detail has path "" and message "Configuration violates the Driver's constraints." The startup formatter preserves that generic detail, so operators cannot identify the failing setting or learn that a canonical absolute path is required. Return a field-specific diagnostic with the path requirements and corrective action, and update the regression assertion accordingly.
  Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against beb627dd5152.

Provenance checked

• Native repository selection and composition changes intended behavior with a stated reason (3115679: Credential Proxy implementation separated repository approval policy from optional native transport; the repair preserves that ownership split while consuming the returned validation errors.)
• Shared configuration admission keeps the original intent (774b3d6: Shared schema and semantic validation must reject invalid configuration before construction and retain bounded field diagnostics.)
• Native transport path constraints keeps the original intent (2fd3d79: The earlier repository credential implementation required absolute canonical paths, prohibited control characters, and bounded Unix socket path length.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Verified introduced repair: The pinned diff replaces discarded validator results with requireConfiguration at both existing entry points and adds a noncanonical CA-path rejection case. (apps/controller/src/drivers/repo/github/installation.ts:41, 87c38f94fb37)
• Bug remains on reviewed main: Both native entry points still ignore returned diagnostics on fetched main; CA loading accepts filesystem-resolvable spellings without enforcing lexical canonicality. (apps/controller/src/composition/repository-credentials/platform.ts:50, beb627dd5152)
• Established contract and provenance: Historical source already required absolute canonical paths and bounded socket names; feat: share discoverable Driver configuration definitions #2980 introduced shared configuration admission, and Credential Proxy Implementation #2487 separated repository policy from native transport. (apps/controller/src/drivers/repository-credentials/github.ts:78, 2fd3d7913258)
• Actionable diagnostic defect: The newly enforced semantic refusal returns an empty detail path and generic constraints message; startup preserves those diagnostics without identifying the native sender setting or corrective action. (apps/controller/src/drivers/repo/github/driver.ts:65, 87c38f94fb37)
• Overlapping open repair: Reject invalid native repository sender paths at startup #3237 repairs the same discarded path-validation results, using a different implementation and broader loader coverage; neither candidate has merged. (d34dec696893)
• Owner direction and accepted startup impact: The collaborator-authored PR explicitly restores strict startup refusal and acknowledges that previously accepted invalid CA paths will fail startup and Helm upgrade checking; discussion contains only bot status comments. (87c38f94fb37)

Likely related people:

• Kevin: Raw commit 774b3d6 adds packages/contracts/src/configuration-definition.ts:392 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 774b3d6760f9; files: packages/contracts/src/configuration-definition.ts)

Labels

Label changes:

• add P2: This focused configuration validation repair affects optional native repository delivery in an operator-managed workflow.
• add merge-risk: 🚨 compatibility: Previously accepted noncanonical CA paths now stop startup; the collaborator-authored PR explicitly accepts restoring that existing contract.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

• P2: This focused configuration validation repair affects optional native repository delivery in an operator-managed workflow.
• merge-risk: 🚨 compatibility: Previously accepted noncanonical CA paths now stop startup; the collaborator-authored PR explicitly accepts restoring that existing contract.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 22, 2026, 8:04 PM ET / October 23, 2026, 00:04 UTC.