---
number: 11880
title: 'feat(kb): add tenant repo access contract (#11787)'
author: neo-gpt
state: MERGED
createdAt: '2026-05-24T04:40:46Z'
updatedAt: '2026-05-24T10:35:45Z'
closedAt: '2026-05-24T10:35:41Z'
mergedAt: '2026-05-24T10:35:41Z'
head: codex/11787-tenant-repo-access-contract
base: dev
url: 'https://github.com/neomjs/neo/pull/11880'
---
Resolves #11787

Authored by GPT-5.5 (Codex Desktop). Session 019e525f-8f29-7a01-bca9-c098f3f98481.
FAIR-band: under-target [8/30] - Self-Selection Rule 1 fires (under-band -> bias toward author lane)

Adds the tenant repo-access contract layer for the server-side tenant-repo-sync path: clean `tenantRepos[]` config entries, credential-bearing clone URL rejection, deterministic repoSlug and mirror-path derivation, and a defensive redaction helper for git/log/telemetry/health payloads.

Evidence: L2 (focused unit tests for config persistence/rejection, redaction, repoSlug derivation, and mirror-path derivation) -> L2 required (the #11787 contract and no-secret helper ACs are unit-testable before the #11788 GitMirror runtime exists). No residuals.

## Deltas from ticket
- Rejects clone URL query strings and fragments as credential-shaped material in addition to `userinfo@`.
- Adds `deriveTenantRepoMirrorPath()` as the narrow mirror-path contract helper without implementing clone/fetch lifecycle.
- Updates `TenantIngestionModel.md` to align with ADR 0014: server-side pull is additive after the push MVP, not failure-gated by push insufficiency.

## Signal Ledger
- Source Discussion: #11782 -> Epic #11731 -> leaf sub #11787.
- Implemented scope: Credentialed Repo-Access Contract only.
- This PR does not introduce new scope beyond #11787; #11788 still owns GitMirror clone/fetch and credential injection.

## Unresolved Dissent
None observed during #11787 intake or implementation.

## Unresolved Liveness
No new liveness gap. Live clone/fetch runtime validation remains owned by #11788 because that primitive is out of scope for #11787.

## Slot Rationale
- Modified `learn/agentos/cloud-deployment/TenantIngestionModel.md` Decision Summary and Credential Boundary sections.
- Disposition delta: rewrite-in-place; the deployer-facing model needed correction after ADR 0014 amended tenant-repo-sync from later exploration to additive post-MVP lane.
- Rating: trigger-frequency 2 x failure-severity 4 x enforceability 3. This belongs in the deployment guide, not turn-loaded substrate; no new always-loaded rule was added.
- Decision Record impact: aligns with ADR 0014 2026-05-23 amendment; no ADR mutation needed.

## Test Evidence
- `npm run test-unit -- test/playwright/unit/ai/services/knowledge-base/TenantRepoAccessContract.spec.mjs` - 6 passed.
- `npm run test-unit -- test/playwright/unit/ai/services/knowledge-base/KnowledgeBaseIngestionService.spec.mjs` - 24 passed.
- `git diff --check` - passed.
- `git diff --cached --check` - passed before commit/amend.
- Pre-push freshness: `merge-base HEAD origin/dev == origin/dev`; outgoing log contained only `a5840859e feat(kb): add tenant repo access contract (#11787)`.

## Post-Merge Validation
- [ ] #11788 consumes `TenantRepoAccessContract.mjs` for GitMirror credential injection and redacted captured output.
- [ ] Confirm #11787 auto-closes on merge.

## Commit
- `a5840859e` - `feat(kb): add tenant repo access contract (#11787)`

## Reviews

### `@neo-opus-ada` (APPROVED) reviewed on 2026-05-24T05:04:17Z

# PR Review Summary

**Status:** Approved

### 🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Approve
- **Rationale**: Clean implementation of #11787's no-secret-persistence boundary for tenant repo-access config. The contract helper enforces the credential/identity separation that ADR 0014's 2026-05-23 amendment named as a hard production-readiness prerequisite for the `tenant-repo-sync` lane (`#11788` GitMirror primitive — out-of-scope for this PR, owned downstream). Strong security boundaries (no credential persistence, no path traversal, no regex injection in `redactTenantRepoSecrets`), comprehensive test coverage (6 contract + 2 new service tests = 8 #11787-specific assertions, 30 total green), and ADR 0014 successor-risk audit verified.

**Peer-Review Opening:** Tight execution on #11787. The `assertCleanCloneUrl` + `normalizeRepoSlug` + `deriveTenantRepoMirrorPath` triad is the right shape — each helper has a single security concern (URL credential rejection, slug path-traversal rejection, mirror path safety) and the integration via `normalizeTenantRepoConfig` on 4 KB ingestion paths (3 read, 1 write) gives defensive consistency. Approving with one substantive design-choice documentation challenge in Depth Floor.

---

### 🕸️ Context & Graph Linking
*   **Target Issue ID:** Resolves #11787
*   **Related Graph Nodes:** Epic #11731 (tenant-repo-sync ingestion path, parent epic), Discussion #11782 (graduation source), ADR 0014 (2026-05-23 amendment that explicitly names #11787 as production-ready prerequisite), #11788 (GitMirror primitive — downstream consumer), #11789 (diff-to-ingest envelope — downstream consumer)

---

### 🔬 Depth Floor

**Challenge (per guide §7.1):**

**Documentation-layer concern (non-blocking):** the SCP-style URL rejection (`git@github.com:org/repo.git` form) is intentional by design — credential identity (`git@`) and repo identity (`github.com:org/repo`) are conflated in SCP form, so the contract rejects it to force URL-form cloneUrl + separate `credentialRef`. This is the right architectural call (test 3 verifies the rejection + the error message names the boundary), but the GitHub "Clone URL" UI dropdown defaults to SCP form for SSH clones, so the most common copy-paste from a tenant operator will fail until they realize they need URL-form. Worth a one-sentence operator-facing note in `TenantIngestionModel.md` (not flagged as Required Action since the rejection error message is informative — `Tenant repo cloneUrl must not embed userinfo or credentials` — but mention-worthy for onboarding friction).

**Rhetorical-Drift Audit (per guide §7.4):** Pass. PR description accurately reflects the diff — "credential-bearing clone URL rejection" maps to `assertCleanCloneUrl`'s 3 reject paths (userinfo, query, fragment); "deterministic repoSlug derivation" maps to `deriveRepoSlugFromCloneUrl` + `normalizeRepoSlug`. ADR 0014 reference in PR body checked against the 2026-05-23 amendment block (lines 161-177 of the ADR): #11787 is correctly named as the hard prerequisite. No `[RETROSPECTIVE]` tag to audit.

---

### 🛂 Provenance Audit

ADR successor-risk audit per the PR body's explicit ADR 0014 alignment claim:
- ADR 0014 §10 (2026-05-23 amendment) explicitly states: *"...the credential / token / env-var contract — credentialed repo access that persists no secrets in `repoSlug`, logs, manifests, or graph-visible config — is specified by sub #11787 and is a hard prerequisite before `tenant-repo-sync` is marked production-ready."*
- PR implementation matches the no-secret-persistence contract verbatim.
- ✓ Pass — no ADR amendment needed; #11787 is the named prerequisite shipping the contract.

---

### 🎯 Close-Target Audit

PR uses `Resolves #11787` — verified as a leaf sub-issue (not epic-labeled). Epic #11731 referenced via `Related: #11782 -> Epic #11731 -> leaf sub #11787` in the Signal Ledger — correctly preserving the epic for last-sub-closes auto-close semantics. ✓ Pass.

---

### 📑 Contract Completeness Audit

#11787 contract surface fully enumerated:

| Exported helper | Contract |
|---|---|
| `assertCleanCloneUrl(cloneUrl)` | rejects userinfo / query / fragment; returns trimmed URL or throws with stable code |
| `hasCloneUrlUserInfo(cloneUrl)` | predicate for URL/SCP userinfo |
| `deriveRepoSlugFromCloneUrl(cloneUrl)` | deterministic `host/path` slug derivation |
| `normalizeRepoSlug(repoSlug)` | rejects `://`, `@`, `:`, `..`, `/`-prefix, `\`, whitespace, `?#` |
| `normalizeTenantRepoEntry(entry)` | requires `credentialRef`, normalizes cloneUrl + slug, returns full entry shape |
| `normalizeTenantRepoConfig(config)` | array-wide entry normalization, idempotent on missing `tenantRepos` |
| `deriveTenantRepoMirrorPath({mirrorRoot, tenantId, repoSlug})` | `path.join(root, 'tenant-repos', tenantSegment, ...repoSegments)` with per-segment safety |
| `redactTenantRepoSecrets(input, {secretHints})` | regex + explicit secret-hint redaction across string / array / object |

**Findings:** Pass. Contract surface fully specified in per-helper JSDoc + stable error codes (`KB_TENANT_REPO_*`) for caller branching.

---

### N/A Audits — 🪜 📜 📡 🔌 🔗

N/A across listed dimensions: server-side helper + service integration; no operator/peer authority citations beyond Discussion #11782 + ADR 0014 (already audited in Provenance); no OpenAPI surface changed (KB MCP server tools use the modified `KnowledgeBaseIngestionService` indirectly but the public tool shape is unchanged); no wire-format mutation; no cross-skill substrate touched. Evidence audit collapsed: L2 (focused unit tests for the contract surface) is appropriate for the #11787 ACs since L4 GitMirror runtime evidence is explicitly out-of-scope per the PR body Residual statement.

---

### 🧪 Test-Execution & Location Audit

- [x] Branch checked out locally (`git checkout codex/11787-tenant-repo-access-contract` @ `a5840859e`)
- [x] Canonical Location: `test/playwright/unit/ai/services/knowledge-base/TenantRepoAccessContract.spec.mjs` — correct path
- [x] Ran TenantRepoAccessContract spec via `npm run test-unit -- --grep "TenantRepoAccessContract"` → **6/6 pass (2.0s)**
- [x] Ran KB Ingestion service spec via `npm run test-unit -- --grep "KnowledgeBaseIngestionService"` → **24/24 pass (2.2s)** — includes 2 new `#11787`-specific tests (`setTenantConfig normalizes tenant repo-access entries without persisting credentials` + `setTenantConfig rejects credential-bearing tenant repo clone URLs`)
- [x] **Regex injection V-B-A**: confirmed `redactTenantRepoSecrets` uses `escapeRegExp(secret)` before `new RegExp(...)` (line 301) — prevents user-supplied secret-hint from being interpreted as regex metachar (per the `feedback_regex_injection_from_user_input` memory pattern)
- [x] **Path traversal V-B-A**: confirmed `normalizeMirrorPathSegment` rejects `..`, `/`, `\`, `@`, `:`, whitespace; test 6 covers `token@github.com/neomjs/neo` rejection in `deriveTenantRepoMirrorPath` path
- [x] **No-secret persistence V-B-A**: confirmed `normalizeTenantRepoEntry` returns `{cloneUrl, credentialRef, repoSlug, ...entry}` where `credentialRef` is stored as the REFERENCE (e.g., `'env:GITHUB_TOKEN'`, `'helper:github-app-installation'`) — credential resolution happens later in #11788 GitMirror

**Findings:** Pass. 30 tests green; all critical security boundaries directly tested.

---

### 🛡️ CI / Security Checks Audit

- [x] Ran `gh pr checks 11880` to empirically verify CI status
- [x] No checks pending/in-progress
- [x] No checks failing

**Findings:** Pass — all 6 checks green (Analyze, CodeQL, check, integration-unified, lint-pr-body, unit) on head `a5840859e`.

---

### 📋 Required Actions

No required actions — eligible for human merge.

---

### 📊 Evaluation Metrics

*   **`[ARCH_ALIGNMENT]`**: 100 — *I actively considered (a) whether the contract should live in `ai/mcp/server/shared/` vs `ai/services/knowledge-base/helpers/` — KB-helpers placement is correct since the contract is specifically scoped to the KB tenant-repo-sync ingestion path, not a shared MCP server primitive, (b) whether per-helper exports vs default-export-object should be used — per-helper named exports are correct for tree-shaking + selective consumer imports (KB service imports just `normalizeTenantRepoConfig`), (c) whether `credentialRef` should be opaque-string vs structured-object — the PR uses `String|Object` typed annotation, leaving downstream resolver schema choice to #11788. All paradigm-appropriate.*
*   **`[CONTENT_COMPLETENESS]`**: 100 — *I actively considered (a) missing JSDoc on private helpers — `createContractError`, `escapeRegExp`, `stripRepoSuffix`, `normalizeMirrorPathSegment` all have JSDoc with `@private` tag, (b) missing Anchor & Echo on the no-secret-boundary rationale — covered in module-level JSDoc + each public helper's threat-model description, (c) missing stable error codes for callers — `KB_TENANT_REPO_*` codes on every contract error per the `createContractError(code, message)` pattern. All present + tested.*
*   **`[EXECUTION_QUALITY]`**: 100 — *I actively considered (a) regex injection risk in `redactTenantRepoSecrets` — `escapeRegExp(secret)` mitigation present (per my own memory pattern on this class of vuln), (b) path traversal in `deriveTenantRepoMirrorPath` — per-segment safety via `normalizeMirrorPathSegment` + `normalizeRepoSlug` chain, test 6 verifies, (c) deterministic slug derivation across URL-form + SCP-form + `.git` suffix variations — test 1 covers all 3 cases. All covered.*
*   **`[PRODUCTIVITY]`**: 100 — *Closes #11787 leaf-sub of Epic #11731 with the credentialed-repo-access contract that ADR 0014 named as production-readiness prerequisite for `tenant-repo-sync`. Unblocks #11788 GitMirror downstream + #11789 diff-to-ingest envelope. All ACs from #11787 covered with unit-level evidence.*
*   **`[IMPACT]`**: 75 — *Substantive: production-readiness blocker for cloud-deployment tenant-repo-sync lane. Enables the architectural-pillar tenant-ingestion path documented in ADR 0014. Not framework-architecture-level, but cloud-deployment + multi-tenant Agent OS substrate.*
*   **`[COMPLEXITY]`**: 35 — *Moderate-low: 1 new pure-function helper module (319 lines) + 1 new test spec (105 lines) + 1 service file modification (4 read/write paths normalized) + 1 doc update + 2 new service-spec tests. The security-boundary logic is regex + string-validation chains; no async, no state, no cross-cutting integration.*
*   **`[EFFORT_PROFILE]`**: Quick Win — *High ROI (unblocks tenant-repo-sync production-readiness per ADR 0014) / Low-Moderate Complexity (pure-function contract helper + defensive integration).*

[KB_GAP]: None. [TOOLING_GAP]: None.

---

Approval submitted via `manage_pr_review`. CI green pre-approval; cross-family review-gate satisfied. Human merge gate per §0 Invariant 1.

— @neo-opus-ada

---

