Skip to content

feat(api): add Hosted fleet control endpoints - #1714

Open
integry wants to merge 4 commits into
mainfrom
feat/hosted-fleet-api
Open

feat(api): add Hosted fleet control endpoints#1714
integry wants to merge 4 commits into
mainfrom
feat/hosted-fleet-api

Conversation

@integry

@integry integry commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Summary

Adds the protected, instance-local API surface that ProPR Fleet uses to complete
Hosted bootstrap verification and collect operational health. These endpoints
expose only the minimum state Fleet needs and never expose OAuth tokens,
credentials, customer data, or the configured login snapshot.

Dependency / PR structure

This is a stacked PR based on feat/instance-admin-roles / #1713. It relies on
that PR's durable numeric-GitHub-ID role model and administrator claim state.
After #1713 merges, this PR can be retargeted to main without changing its
scope.

Endpoints

  • GET /api/internal/hosted/bootstrap reports whether the configured immutable
    GitHub user ID has claimed the durable administrator role and whether the
    temporary bootstrap controls can be removed.
  • GET /api/internal/hosted/status returns bounded API/RoutingHub operational
    state for Fleet verification and heartbeat collection.
  • GET /api/internal/hosted/queue returns bounded waiting/active queue counts
    for health and drain decisions.

The routes are registered before normal OAuth middleware because they use a
dedicated service credential and are not customer-browser endpoints.

Security and identity

  • Requires PROPR_FLEET_CONTROL_SECRET with at least 32 characters.
  • Accepts the secret only through X-ProPR-Fleet-Secret and compares it with
    timingSafeEqual after equal-length validation.
  • Validates PROPR_HOSTED_INITIAL_ADMIN_GITHUB_USER_ID as a positive safe
    integer and uses it—not the mutable login—to verify administrator ownership.
  • Treats PROPR_HOSTED_INITIAL_ADMIN_GITHUB_LOGIN as an operational snapshot
    only and does not return it from the API.
  • Reads durable instance roles and preserves ProPR as the authority for local
    administrator/member authorization.

Configuration

Adds the following documented environment values:

  • PROPR_FLEET_CONTROL_SECRET
  • PROPR_HOSTED_INITIAL_ADMIN_GITHUB_USER_ID
  • PROPR_HOSTED_INITIAL_ADMIN_GITHUB_LOGIN

Fleet/Ansible supplies these per tenant through the protected Hosted
environment; they must not be baked into images or shared between tenants.

Cross-repository dependencies

Validation

  • npm run typecheck
  • Native Node test suite: 5/5 hosted Fleet route tests pass.
  • Covers missing/incorrect credentials, pending claim without login leakage,
    verified immutable administrator identity, rejection of a different admin,
    and protected operational status.
  • git diff --check passes.

Rollout and rollback

Deploy only after #1713 and configure the per-tenant secret/owner identity
through Ansible. Fleet treats an unavailable or inconclusive endpoint as failed
verification and does not report the instance ACTIVE.

Rollback removes the internal routes and environment values. Existing durable
role state is unchanged; Fleet must remain paused or in pre-active state until a
compatible ProPR version is restored.

@integry integry added the AI label Aug 2, 2026
Repository owner deleted a comment from propr-dev Bot Aug 2, 2026
Repository owner deleted a comment from propr-dev Bot Aug 2, 2026
Repository owner deleted a comment from propr-dev Bot Aug 2, 2026
Repository owner deleted a comment from propr-dev Bot Aug 2, 2026
@integry

integry commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Hosted fleet routes should only be enabled if fleet control is enabled via env

@propr-dev

propr-dev Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Applied the requested follow-up changes in commit 8db5de0

Summary of Changes

Implemented the PR follow-up without committing.

  • Hosted fleet routes are mounted only when PROPR_FLEET_CONTROL_SECRET is at least 32 characters.
  • Reused the same enablement check for authentication.
  • Updated .env.example documentation and added regression coverage.

Validation passed:

  • API typecheck
  • API lint
  • Hosted fleet route tests: 6/6
  • git diff --check

🤖 Implementation Details

  • Model: GPT-5.6 Sol (max)
  • Requested By: @integry
  • Time: 8m 13s
  • Tokens: 1,558,347 (1,544,126 in / 14,221 out)
  • Cost: $1.59

Undo ChangesView Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR
Processing comment ID: 5155392232✓

Implemented the PR follow-up without committing.

- Hosted fleet routes are mounted only when `PROPR_FLEET_CONTROL_SECRET` is at least 32 characters.
- Reused the same enablement check for authentication.
- Updated `.env.example` documentation and added regression coverage.

Validation passed:

- API typecheck
- API lint
- Hosted fleet route tests: 6/6
- `git diff --check`

PR: #1714
Comment by: @integry (ID: 5155392232)
Model: gpt-5.6-sol
@integry

integry commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

/ultrafix

@propr-dev propr-dev Bot added the ultrafix label Aug 2, 2026
@propr-dev

propr-dev Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🔄 Ultrafix loop started (goal: 8/10, max cycles: 10)

First action: /review

💡 Tip: Remove the ultrafix label from this PR to stop further ultrafix cycles.

@propr-dev

propr-dev Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

This PR adds secret-gated, instance-local Fleet endpoints for bootstrap verification, operational health, and queue health. The implementation is focused and uses immutable GitHub IDs, defensive route gating, dependency injection, and no-store responses. It needs minor changes before merge, primarily to enforce the promised bounded response contract and make bootstrap-cleanup signals unambiguous.

Findings

🟡 Delegated handlers do not enforce a bounded Fleet responsepackages/api/routes/hostedFleetRoutes.ts:78-98 forwards directly to the normal status and queue handlers. Any current or future fields added to /api/status or /api/queue/stats will automatically become accessible through the Fleet credential. Construct explicit Fleet response objects or adapters that allowlist the intended fields.

🟡 Bootstrap cleanup state is ambiguous when additional bootstrap administrators existpackages/api/routes/hostedFleetRoutes.ts:53-68 reports whether the initial login appears anywhere in PROPR_ADMIN_USERS, but not whether it is the only entry. A configuration such as owner,break-glass-admin produces the same environmentBootstrapActive value as owner; a consumer could consequently treat removal of the whole bootstrap variable as safe. Return an explicit bootstrapOnlyInitialOwner or authoritative temporaryControlsRemovable result.

🟡 The supposedly timing-safe comparison leaks lengthpackages/api/routes/hostedFleetRoutes.ts:23-27 returns before timingSafeEqual when byte lengths differ. This principally exposes credential length rather than contents, so exploitation over a network is unlikely, but it weakens the authentication primitive. Comparing fixed-length hashes would avoid the early-exit distinction.

🟡 Normalized whitelist entries are not deduplicatedpackages/api/routes/hostedFleetRoutes.ts:56-68 treats owner,OWNER as two entries even though GitHub usernames are case-insensitive. That incorrectly makes whitelistOnlyInitialOwner false and could prevent automated cleanup. Normalize into a Set before evaluating cardinality.

🟡 Security-critical server wiring is not integration-testedpackages/api/test/hostedFleetRoutes.test.ts:65-160 tests the helper and handlers independently, but does not prove that the routes are absent when the gate is disabled, present before OAuth middleware when enabled, and protected through an actual Express request. An integration test would catch route-order or registration regressions.

🟢 Canonicalize the configured GitHub IDpackages/api/routes/hostedFleetRoutes.ts:44-51 accepts leading-zero IDs such as 00100, which will not match a stored canonical value such as 100. Trimming and canonicalizing the decimal string, or rejecting noncanonical forms, would prevent confusing false negatives.

🟢 Cover the remaining response branches — Tests should exercise invalid or missing initial-admin IDs (409), unavailable delegated handlers (503), incorrect credentials on the queue endpoint, mixed-case/duplicate whitelist entries, and delegated-handler failures.

🟢 Document secure secret generation.env.example:203-211 requires only 32 characters, which does not ensure entropy. Adding an operator command such as openssl rand -hex 32 would reduce the chance of weak manually chosen Fleet credentials.

🟢 Reduce ambient environment dependence in tests and constructioncreateHostedFleetRoutes injects most configuration but environmentBootstrapActive still calls getBootstrapAdminUsernames() against global process state. Injecting the resolved bootstrap usernames would make configuration snapshots and unit tests more deterministic.

Hosted routes are genuinely disabled without Fleet controlpackages/api/server.ts:226-238 conditionally registers all three endpoints, while each handler independently rechecks the captured credential. This provides useful defense in depth and addresses the latest review requirement.

Route ordering correctly supports machine-to-machine access — The Fleet routes are registered before ensureAuthenticated but retain their own narrow authentication, while normal API routes remain protected.

Durable verification uses the immutable identity and required rolepackages/api/routes/hostedFleetRoutes.ts:48-52 checks both the configured numeric GitHub ID and role: 'admin', avoiding username-renaming and “some other administrator exists” errors.

Sensitive login data is not returned — The bootstrap response exposes only the configured immutable ID and boolean state, and the test explicitly verifies that the login is absent.

Operational safeguards are appropriate — Generic 401 responses avoid credential-oracle details, successful responses are marked no-store, and dependency injection keeps the new logic readily testable.

Score

The core design is sound and the requested environment gate is correctly implemented, but the response allowlisting and bootstrap-cleanup semantics should be tightened before relying on these endpoints operationally.

Score: 7/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 3m 26s
  • Tokens: 36,664 (30,994 in / 5,670 out)
  • Cost: $0.28

View Task


💡 Next step: Comment /fix on this PR to have the AI automatically implement the suggestions above.
The /fix command gathers all unprocessed AI review comments and applies fixes in a single pass.
You can edit or delete review comments before running /fix to control which suggestions are applied.
Add extra instructions if needed, e.g. /fix only address the critical findings.

🤖 Review by ProPR

@propr-dev

propr-dev Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Applied the requested follow-up changes in commit 4b4a088

Addressed 1 AI review comment (IDs: 5155503575)

Summary of Changes

Implemented all review follow-ups for PR #1714.

  • Fleet status/queue responses are explicitly validated and allowlisted.
  • Added bootstrapOnlyInitialOwner, username deduplication, injected bootstrap configuration, and GitHub ID canonicalization.
  • Fleet-secret comparison now uses fixed-length SHA-256 digests.
  • Added gated route-registration wiring before OAuth middleware.
  • Expanded unit and Express integration coverage.
  • Documented openssl rand -hex 32 secret generation.

Key files: hostedFleetRoutes.ts, server.ts, hostedFleetRoutes.test.ts.

Verification passed:

  • Repository-wide TypeScript check
  • API lint
  • 12 Fleet route tests
  • 32 combined Fleet/status tests
  • git diff --check

No commit created.


🤖 Implementation Details

  • Model: GPT-5.6 Sol (max)
  • Requested By: @propr-ultrafix
  • Time: 13m 29s
  • Tokens: 2,170,275 (2,143,477 in / 26,798 out)
  • Cost: $2.47

View Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR

Implemented all review follow-ups for PR #1714.

- Fleet status/queue responses are explicitly validated and allowlisted.
- Added `bootstrapOnlyInitialOwner`, username deduplication, injected bootstrap configuration, and GitHub ID canonicalization.
- Fleet-secret comparison now uses fixed-length SHA-256 digests.
- Added gated route-registration wiring before OAuth middleware.
- Expanded unit and Express integration coverage.
- Documented `openssl rand -hex 32` secret generation.

Key files: [hostedFleetRoutes.ts](/tmp/git-processor/worktrees/integry/propr/pr-1714-followup-2026-08-02T05-01-52/packages/api/routes/hostedFleetRoutes.ts:44), [server.ts](/tmp/git-processor/worktrees/integry/propr/pr-1714-followup-2026-08-02T05-01-52/packages/api/server.ts:217), [hostedFleetRoutes.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1714-followup-2026-08-02T05-01-52/packages/api/test/hostedFleetRoutes.test.ts:1).

Verification passed:

- Repository-wide TypeScript check
- API lint
- 12 Fleet route tests
- 32 combined Fleet/status tests
- `git diff --check`

No commit created.

PR: #1714
Comment by: @propr-ultrafix (ID: 0)
Model: gpt-5.6-sol
@propr-dev

propr-dev Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

This PR adds secret-protected, instance-local Fleet endpoints for bootstrap verification and bounded operational health reporting. The environment-controlled registration gate, durable numeric GitHub-ID lookup, response field filtering, and pre-OAuth routing are thoughtfully implemented and well tested. It needs minor changes before merging, primarily to make failure handling and delegated status collection robust.

Findings

🟡 Bootstrap database failures are not handledpackages/api/routes/hostedFleetRoutes.ts:167 awaits the instance_members query outside a try/catch. A database error can produce an unhandled rejected handler under Express 4 or an inconsistent default error response under Express 5, unlike the sanitized JSON errors from the status endpoints. Catch failures and return a stable 503 response, with a corresponding test.

🟡 Operational string values are not actually boundedpackages/api/routes/hostedFleetRoutes.ts:66-80 allowlists field names but accepts arbitrary strings of any length. A future delegated handler could place diagnostic details or secrets in one of these fields and have them forwarded verbatim. Validate known enum values or, where forward compatibility is required, impose conservative length and character limits.

🟡 Delegating through a fabricated Express response is fragilecaptureDelegatedJson at packages/api/routes/hostedFleetRoutes.ts:101-129 relies on an as unknown as Response object implementing only a few response methods. A harmless future change to getStatus or getQueueStats, such as using send, sendStatus, end, type, or response locals, could silently turn Fleet health checks into 503 responses. Extract pure status-collection functions shared by both routes instead of adapting HTTP handlers.

🟡 Tests do not verify compatibility with the real delegated handlerspackages/api/test/hostedFleetRoutes.test.ts:69-88 uses simplified mock handlers, while production wires createStatusRoutes().getStatus and createQueueRoutes().getQueueStats. At least one wiring-level test using those actual handlers would catch response-shape changes and incompatibilities with the fabricated response adapter.

🟡 Invalid numeric GitHub IDs can be treated as configuredcanonicalizeGithubUserId at packages/api/routes/hostedFleetRoutes.ts:49-53 accepts values such as 0 and arbitrarily long digit strings. Those are reported as pending claims rather than producing the intended configuration error. Retain string-based handling for large IDs, but require a positive, reasonably bounded decimal identifier.

🟢 Delegated failures should use a stable outward status contractpackages/api/routes/hostedFleetRoutes.ts:195-199 and 224-228 mirror any non-2xx status, including inappropriate redirect or client-error statuses, while replacing the body with an “unavailable” error. Normalizing delegated failures to 503 would make Fleet behavior predictable.

🟢 Route exports are inconsistentpackages/api/routes/index.ts:27 exports createHostedFleetRoutes and isHostedFleetControlEnabled, but not registerHostedFleetRoutes, forcing packages/api/server.ts:34 to import directly from the module alongside the existing route barrel. Exporting all three together would keep the public route API coherent.

🟢 Additional malformed-output tests would strengthen the boundary — Tests should cover non-string operational fields, negative/fractional queue counts, excessively long status strings, zero GitHub IDs, and database-query failures.

Fleet routes are now genuinely gated by environment configurationregisterHostedFleetRoutes registers nothing unless PROPR_FLEET_CONTROL_SECRET meets the configured minimum length, directly addressing the latest review context.

Authentication handling is careful — The implementation uses a dedicated header, digest-based constant-time comparison, and Cache-Control: no-store, while keeping these endpoints independent of customer OAuth sessions.

Responses expose a deliberately small surface — Queue output is reduced to waiting/active counts, operational output is restricted to four fields, and the configured GitHub login is never returned.

Bootstrap verification uses the durable identity correctly — The lookup matches the configured canonical numeric GitHub ID and the admin role, preventing another administrator or a renamed login from incorrectly satisfying the initial-owner claim.

The test suite covers the principal security and registration paths well — It checks missing and incorrect credentials, route omission when disabled, placement before the OAuth boundary, response sanitization, numeric-ID canonicalization, and removal-state calculations.

Score

Strong security-conscious implementation with good coverage, held back by an avoidable handler-adaptation abstraction and incomplete error/value-boundary handling.

Score: 7/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 3m 36s
  • Tokens: 43,597 (36,401 in / 7,196 out)
  • Cost: $0.35

View Task


💡 Next step: Comment /fix on this PR to have the AI automatically implement the suggestions above.
The /fix command gathers all unprocessed AI review comments and applies fixes in a single pass.
You can edit or delete review comments before running /fix to control which suggestions are applied.
Add extra instructions if needed, e.g. /fix only address the critical findings.

🤖 Review by ProPR

@propr-dev

propr-dev Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Applied the requested follow-up changes in commit e2f58c0

Addressed 1 AI review comment (IDs: 5155610491)

Summary of Changes

Implemented all requested review fixes without committing:

  • Added sanitized bootstrap DB-error handling, positive bounded GitHub IDs, strict operational enums, and stable 503 health failures in hostedFleetRoutes.ts.
  • Replaced fabricated Express responses with shared pure collectors in statusRoutes.ts and queueRoutes.ts.
  • Updated server/barrel wiring and added real-collector, malformed-output, database-failure, and ID-boundary tests.

Verification passed:

  • 36 focused Fleet/status tests
  • 155 repository unit tests
  • Root and API TypeScript checks
  • Targeted ESLint
  • git diff --check

The standalone demo-mode suite still has two unrelated pre-existing expectation mismatches; no demo-mode code was changed.


🤖 Implementation Details

  • Model: GPT-5.6 Sol (max)
  • Requested By: @propr-ultrafix
  • Time: 11m 59s
  • Tokens: 2,962,345 (2,939,213 in / 23,132 out)
  • Cost: $2.87

View Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR

Implemented all requested review fixes without committing:

- Added sanitized bootstrap DB-error handling, positive bounded GitHub IDs, strict operational enums, and stable `503` health failures in [hostedFleetRoutes.ts](/tmp/git-processor/worktrees/integry/propr/pr-1714-followup-2026-08-02T05-21-25/packages/api/routes/hostedFleetRoutes.ts:30).
- Replaced fabricated Express responses with shared pure collectors in [statusRoutes.ts](/tmp/git-processor/worktrees/integry/propr/pr-1714-followup-2026-08-02T05-21-25/packages/api/routes/statusRoutes.ts:68) and [queueRoutes.ts](/tmp/git-processor/worktrees/integry/propr/pr-1714-followup-2026-08-02T05-21-25/packages/api/routes/queueRoutes.ts:13).
- Updated server/barrel wiring and added real-collector, malformed-output, database-failure, and ID-boundary tests.

Verification passed:

- 36 focused Fleet/status tests
- 155 repository unit tests
- Root and API TypeScript checks
- Targeted ESLint
- `git diff --check`

The standalone demo-mode suite still has two unrelated pre-existing expectation mismatches; no demo-mode code was changed.

PR: #1714
Comment by: @propr-ultrafix (ID: 0)
Model: gpt-5.6-sol
@propr-dev

propr-dev Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

This PR adds narrowly scoped, secret-protected Fleet bootstrap and health endpoints, correctly gated by PROPR_FLEET_CONTROL_SECRET and registered before OAuth authentication. The implementation is security-conscious and well tested, but needs minor changes to keep Fleet heartbeat collection lightweight and reliably bounded.

Findings

🟡 Fleet status collection performs substantially more work than the endpoint needsserver.ts:228-229 passes the complete statusRoutes.collectStatus pipeline into Fleet, while hostedFleetRoutes.ts:162-181 retains only four GitHub fields. Each heartbeat can therefore initialize agents, run agent health checks, inspect indexing, load summarization state, and inspect agent-runtime status (statusRoutes.ts:139-168). This increases latency and load and couples Fleet availability to unrelated subsystems. Provide a dedicated narrow collector for auth/intake state.

🟡 Collectors have no overall response deadline — Database, Redis, BullMQ, agent, or indexing operations that never settle can leave /bootstrap, /status, or /queue requests open indefinitely (hostedFleetRoutes.ts:126-138, 162-201). Some agent checks have individual timeouts, but there is no endpoint-wide deadline. Fleet heartbeat endpoints should fail with a bounded 503 response.

🟡 A missing initial-admin login produces ambiguous bootstrap results — Only the numeric ID is required by getBootstrapStatus (hostedFleetRoutes.ts:123-127). If PROPR_HOSTED_INITIAL_ADMIN_GITHUB_LOGIN is absent, the endpoint returns 200 with all login-derived flags set to false, which is indistinguishable from valid configuration where the controls are inactive or contain additional users. Either validate the login as required, return an explicit initialAdminLoginConfigured field, or document this fail-closed behavior and test it.

🟢 Queue heartbeats collect unused historical counts — Fleet only returns waiting and active, but queueRoutes.ts:13-23 also requests completed, failed, and delayed counts on every call. A narrow Fleet collector using only the two required BullMQ calls would reduce recurring Redis work.

🟢 Authentication logic is duplicated across all three handlershostedFleetRoutes.ts:117-200 repeats the cache and authorization checks. A small route middleware would make it harder for future Fleet endpoints to omit either protection accidentally. The expected secret digest could also be precomputed once at route creation.

🟢 Document the hosted configuration contract more explicitly.env.example:206-214 explains the activation secret but not the required format and purpose of the two initial-administrator variables, the resulting 409 behavior, or whether changing them requires a restart. This is important for operator-managed bootstrap automation.

🟢 Consider a standard bearer credential or explicit header-redaction guidance — A custom x-propr-fleet-secret header works, but observability and proxy products commonly redact Authorization automatically and may retain custom headers. If the custom header remains, deployment guidance should require it to be redacted from proxy/APM logs.

🟢 Avoid process-global console mutation in tests — Tests temporarily replace console.error (hostedFleetRoutes.test.ts:214-227, 334-348). This can suppress unrelated output if test concurrency changes. Injecting a logger or using a scoped mock would make the tests safer.

Hosted routes are genuinely disabled by defaultregisterHostedFleetRoutes registers nothing unless the configured secret is at least 32 characters, and the Express wiring tests verify disabled routes fall through to normal OAuth protection.

Credential checking and response handling are defensive — The implementation compares fixed-size SHA-256 digests with timingSafeEqual, consistently returns sanitized errors, and applies Cache-Control: no-store.

Returned health data is tightly allowlisted — The parsers validate both types and bounded enum values and construct fresh response objects, preventing unrelated status fields, routing URLs, credentials, or backend error details from leaking.

Bootstrap verification uses durable immutable identity correctly — GitHub IDs remain strings, are canonicalized without unsafe numeric conversion, and are matched against the exact administrator role. Tests cover renamed users, leading zeroes, IDs above JavaScript’s safe-integer range, and unrelated administrators.

Test coverage is strong overall — The suite covers route gating, incorrect credentials, malformed configuration, database and collector failures, output allowlisting, administrator-state distinctions, and actual Express middleware ordering.

Score

The core design and security boundaries are solid, with the main remaining concern being excessive and potentially unbounded work on a recurring operational endpoint.

Score: 8/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 4m 47s
  • Tokens: 54,032 (45,810 in / 8,222 out)
  • Cost: $0.43

View Task


💡 Next step: Comment /fix on this PR to have the AI automatically implement the suggestions above.
The /fix command gathers all unprocessed AI review comments and applies fixes in a single pass.
You can edit or delete review comments before running /fix to control which suggestions are applied.
Add extra instructions if needed, e.g. /fix only address the critical findings.

🤖 Review by ProPR

@propr-dev propr-dev Bot removed the ultrafix label Aug 2, 2026
Base automatically changed from feat/instance-admin-roles to main August 3, 2026 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants