feat(server): infrastructure device integration foundation — facility fans first (1/5)

[rongxin-liu]

Reviewable diff: +2452/-59 across 31 files (excludes generated, test, and story files; 513 of the additions are the committed plan doc).

Summary

This is the foundational PR for integrating external infrastructure devices into Proto Fleet: equipment on the facility side (fans, and later PDUs, dampers, pumps, ...) that the server can inventory and eventually command, without the core ever learning device protocols. It delivers the persisted infrastructure_device concept, a protocol-agnostic driver adapter registry, and a permission-gated Connect CRUD API. Facility fans (a single fan or fan group behind a PLC/drive) are the first device kind, and Modbus TCP is the first driver adapter — motivated by curtailment sequencing, but the foundation is deliberately kind- and protocol-agnostic.

Stack: PR 1 of 5 per the committed plan (docs/plans/2026-07-09-facility-fan-curtailment-integration-plan.md, tracked by #723). The fan use case continues in: PR 2 client add/edit UI, PR 3 response-profile fan settings, PR 4 Modbus write path + simulator, PR 5 reconciler sequencing (fans off after miners confirm curtailed; fans on before miners restore). No device behavior activates from this PR — the write path (SetState) intentionally returns not-implemented, so this is inert surface area. Diff is against main.

How it works

An operator with site:manage on the target site calls InfrastructureService.CreateInfrastructureDevice with protocol-blind fields (site, building, name, device kind, fan count) plus a driver_type key and an opaque driver_config JSON blob. The domain service validates the blind fields itself, then resolves the adapter for driver_type from the driver registry and delegates blob validation to it — the core never parses protocol config. The Modbus TCP adapter enforces unit ID 1-247, port bounds, register address 0-65535, write mode coil/holding_register, and fails closed on endpoints: only private RFC1918 / IPv6 ULA IP literals are accepted — loopback, link-local (incl. IMDS), multicast, broadcast, unspecified, and public addresses are all rejected, since the server will eventually open raw, unauthenticated Modbus writes to the stored address. Validation errors never echo the submitted endpoint, so a near-miss OT address cannot leak into request logs.

All five RPCs authorize against the device's site (ResourceContext{SiteID}), not the bare org: Create checks the request's site; Get/Update/Delete resolve the device under org scope and then authorize its current site (Update also checks the target site on a move); List pushes the caller's readable-site set into the SQL filter — EffectivePermissions.SiteScopeFor projects site:read authority as an allowlist (site-scoped grants) or a denylist (org grant narrowed away at specific sites), so unreadable rows are never fetched and an empty readable set short-circuits without touching the DB. Update/Delete additionally predicate the SQL write on the site the caller was authorized against (expected_site_id in the WHERE clause), so a concurrent site move between authorization and write fails closed as NotFound. driver_config — the OT control topology — is returned only to site:manage holders (per device in List and on Get); all five procedures are in SensitiveBodyProcedures (bodies stay out of debug logs) and SessionOnlyProcedures (a leaked API key cannot reach the OT control surface). Every mutation emits an infrastructure_device.created/.updated/.deleted activity event after commit, carrying protocol-blind metadata only (never driver_config).

Writes run in a transaction that row-locks the parent site (LockSiteForWrite), closing the check-then-write race against concurrent DeleteSite; a device move locks both the source and target sites (ascending ID order, deadlock-free), so a move out of a site serializes against that site's deletion instead of slipping a live device past the cascade. DeleteSite's cascade now also soft-deletes the site's infrastructure devices, with the count surfaced through DeleteSiteResponse, the ListSites projection, and the client's delete-confirmation dialog. Rows persist to infrastructure_device with org/site scoping identical to buildings; reads gate on site:read.

Extensibility contract (the point of this PR):

• New protocol (SNMP, HTTP, BACnet, ...): one adapter package + one registry line + a client form module. No schema, proto, handler, or service changes — driver_config crosses the wire as an opaque JSON string with no per-protocol messages.
• New device kind (e.g. PDU): a migration extending the kind CHECK constraints, additive edits to the proto validation list and models.ValidKind, plus the adapter/form work above. device_kind is a string on the wire precisely so this is never a breaking change. Sub-component control (e.g. PDU outlets) fits by modeling each outlet as its own device row — driver_config already carries arbitrary addressing.
• New capability (e.g. fan speed): SetState takes a struct-shaped DesiredState (power mode today) so fields like SpeedPercent are additive; Capabilities() lets the UI gate per-driver features. ReadStatus is doc-reserved for v2 status read-back.

Diagrams

flowchart LR
  Client["Connect client"] --> Handler["InfrastructureService handler: site-scoped RBAC + driver_config redaction"]
  Handler --> Svc["infrastructure.Service (protocol-blind validation)"]
  Svc --> Registry["driver.Registry: driver_type to adapter"]
  Registry --> Modbus["modbustcp adapter (first): ValidateConfig (bounds + RFC1918/ULA endpoint guard)"]
  Registry --> Future["future adapters: snmp, http, bacnet, ..."]
  Svc --> Tx["Tx: LockSiteForWrite + insert/update predicated on expected_site_id"]
  Tx --> DB[("infrastructure_device")]
  Svc --> Audit["activity log: created/updated/deleted (no driver_config)"]

Loading

flowchart TD
  A["PR 1: device foundation (this PR)"] --> B["PR 2: client add/edit UI"]
  A --> C["PR 3: profile fan settings"]
  A --> D["PR 4: Modbus write path"]
  C --> E["PR 5: reconciler fan sequencing"]
  D --> E

Loading

Review guide: files by layer

Grouped by layer, in suggested reading order. The four bold files carry the design and security decisions; the rest mostly mirror established buildings/sites conventions.

1. Wire contract — the extensibility seam

File	What to look at
proto/infrastructure/v1/infrastructure.proto	The whole API shape: device_kind/driver_type as strings, driver_config as an opaque JSON string, the redaction contract documented on the field, optional enabled with presence tracking on both Create (omitted = true) and Update (omitted = preserve current)
proto/sites/v1/sites.proto	Additive only: infrastructure_device_count on SiteWithCounts and deleted_infrastructure_device_count on DeleteSiteResponse

2. Schema & queries

File	What to look at
server/migrations/000122_create_infrastructure_device_table.{up,down}.sql	Table shape: org/site composite FK, partial unique (site, name), CHECK constraints on kind/fan_count, updated_at trigger — mirrors building (000044)
server/sqlc/queries/infrastructure_device.sql	CRUD queries; site_ids/excluded_site_ids list filters (the caller's readable-site set lands here); expected_site_id in the Update/Delete WHERE clauses (stale-site fail-closed guard); soft-delete RETURNING for the audit stamp
server/sqlc/queries/site.sql	ListSites count subquery + the cascade soft-delete statement

3. Driver adapter layer — the protocol-agnostic boundary

File	What to look at
server/internal/domain/infrastructure/driver/driver.go	The Controller interface (SetState, ValidateConfig, Capabilities), the driver_type registry, and the ReadStatus reservation — the seam every future protocol plugs into
server/internal/domain/infrastructure/driver/modbustcp/modbustcp.go	Config schema + validation bounds; the fail-closed RFC1918/ULA endpoint guard (no loopback/link-local/IMDS/public); endpoint-free error messages; the dial-time-allowlist SECURITY PRECONDITION on the stubbed SetState

4. Domain service

File	What to look at
server/internal/domain/infrastructure/service.go	Protocol-blind validation/normalization, per-kind rules, tx + LockSiteForWrite (source and target site on a move, ascending-ID order via siteLockOrder), registry delegation, post-commit audit events (metadata never includes driver_config), the PR 3 delete-guard TODO
server/internal/domain/infrastructure/models/models.go	Domain shapes; ExpectedSiteID semantics on UpdateParams

5. Persistence

File	What to look at
server/internal/domain/stores/interfaces/infrastructure_device.go, server/internal/domain/stores/sqlstores/infrastructure_device.go	Store contract + sqlc-backed impl: shared deviceFromRow mapper, empty config normalized to {}, org-scoped queries only
server/internal/domain/stores/interfaces/site.go, server/internal/domain/stores/sqlstores/site.go	One added cascade method: SoftDeleteInfrastructureDevicesBySite

6. API surface & security gates

File	What to look at
server/internal/handlers/infrastructure/handler.go	The permission surface: per-site authorization on all five RPCs (resolve-then-authorize on Get/Update/Delete, both-sites check on a move), List's readable-site filter composition (allowlist intersection / denylist, empty-set short-circuit, per-device fail-closed backstop), driver_config redaction via middleware.HasPermission (fail-closed on wiring errors), ExpectedSiteID threading
server/internal/domain/authz/effective.go	New SiteScopeFor(key) accessor: projects narrowing semantics into an allowlist/denylist a SQL query can consume
server/internal/handlers/middleware/permission.go	New SiteScopeForPermission helper wrapping the accessor with the standard internal-actor short-circuit and fail-closed handling
server/internal/handlers/infrastructure/translate.go	Proto ↔ domain mapping; omitted enabled defaults to true on Create; on Update the presence pointer passes through to the store so preservation happens in the UPDATE itself
server/internal/handlers/middleware/rpc_permissions.go	RBAC map entries: reads site:read, writes site:manage
server/internal/handlers/interceptors/config.go	All five procedures in SensitiveBodyProcedures (bodies out of debug logs) and SessionOnlyProcedures (no API-key access to the OT control surface)
server/cmd/fleetd/main.go	Wiring only: store/service construction + mux mount + reflection entry

7. Site delete cascade (server + client)

File	What to look at
server/internal/domain/sites/service.go, server/internal/domain/sites/models/models.go	Cascade soft-deletes infrastructure devices inside the existing DeleteSite tx; count carried into the result and audit entry
server/internal/handlers/sites/handler.go, server/internal/handlers/sites/translate.go	Counts mapped through to the proto responses
client/src/protoFleet/api/sites.ts	DeleteSiteCounts carries deletedInfrastructureDeviceCount through the deleteSite hook
client/src/protoFleet/features/sites/components/SiteDeleteDialog/SiteDeleteDialog.tsx	infrastructureDeviceCount included in the cascade gate and summary ("deleted", not "unassigned")

8. Tests, generated code, docs

File	What to look at
*_test.go, SiteDeleteDialog.test.tsx	Skim for coverage claims (see Testing & validation below); service_integration_test.go and handler_test.go pin the security behaviors
server/internal/testutil/database_setup.go	Test-harness hardening only (no production impact): per-test database teardown now waits out and retries transient Postgres crash-recovery blips (57P03, unexpected EOF, connection refused) instead of failing green tests, matching the retry the setup path already had
server/generated/**, client/src/protoFleet/api/generated/**	Generated via just gen — skip
docs/plans/2026-07-09-facility-fan-curtailment-integration-plan.md	The reviewed plan doc — context for the 5-PR series

Key technical decisions & trade-offs

• Opaque driver_config (jsonb + JSON string on the wire) over typed per-protocol proto messages — new protocols need no proto/schema change; trade-off: server-side validation is the only schema enforcement, so it lives in the adapter.
• In-process adapter registry shaped like a future go-plugin service over reusing the miner plugin system — the miner Driver contract is mining-specific; the interface is extractable to a subprocess later (e.g. site-local Modbus RTU on fleetnode) without touching callers.
• device_kind as a constrained string, not a proto enum — new kinds (PDU, damper, pump) are additive validation-list edits, never wire-format changes. The fan-specific bits (fan_count, kind CHECKs) are deliberately named and contained, to be generalized when a second kind arrives rather than speculatively abstracted now.
• Endpoint guard fails closed: private RFC1918 / IPv6 ULA IP literals only (no hostnames) — loopback, link-local (incl. IMDS 169.254.169.254), multicast, broadcast, unspecified, and public addresses are all rejected, removing the SSRF/local-pivot surface by construction before any write path exists. This is a save-time bound, not dial-time authorization: the write path (PR 4) must enforce a per-site commissioned control-subnet allowlist at dial time, recorded as a SECURITY PRECONDITION on modbustcp.SetState and in the plan doc.
• Site-scoped RBAC with fail-closed write predicates — all RPCs authorize against the device's site rather than the bare org (same resolve-then-authorize pattern and accepted same-org existence leak as buildings.GetBuildingStats); Update/Delete carry the authorized site into the SQL WHERE clause so a concurrent site move invalidates the mutation. Response-widening probes use middleware.HasPermission, so an authz wiring failure surfaces as Internal instead of silently redacting.
• List authorization filters in SQL, not in the handler — EffectivePermissions.SiteScopeFor + middleware.SiteScopeForPermission project the caller's site:read authority (narrowing semantics included) into SiteIDs/ExcludedSiteIDs on the query, over fetching the whole org and dropping rows per-device. Sets the pattern for future site-scoped list endpoints; a per-device in-memory check remains as a zero-cost fail-closed backstop.
• InfrastructureService is session-only — a leaked API key with site permissions must not be able to read or rewrite OT control topology; no API-key automation consumes this service (the PR 5 reconciler runs in-process and bypasses transport interceptors).
• Update's omitted enabled preserves the current value, atomically in SQL — not "default true": an unrelated update must neither silently disable nor silently re-enable a device. Presence travels to the write as a nullable param (COALESCE(narg('enabled'), enabled) in the UPDATE), so there is no read-then-write window against a concurrent toggle. Explicit values always win.
• Site row-lock in the write tx instead of an existence pre-check — same TOCTOU fix buildings received; a concurrent DeleteSite can't strand a device on a soft-deleted site. A move additionally locks the source site in ascending-ID order with the target, so it serializes against the source's DeleteSite cascade and crossing moves cannot deadlock.
• Audit via post-commit activity events — create/update/delete emit protocol-blind metadata only (never driver_config); the delete event stamps from the soft-delete's own UPDATE … RETURNING row (mirrors SoftDeleteBuilding), so the audit reflects the device actually deleted even under a concurrent rename/move.
• Deletion reference-guard deferred to PR 3 — facility_fan_device_ids doesn't exist yet; a FailedPrecondition guard mirroring the response-profile/automation-rule pattern lands with that column (TODO recorded in code).

Testing & validation

• Postgres integration tests (docker-compose, unique DB per test): CRUD lifecycle, fan-count normalization, trimmed driver_type persistence, site-filter discrimination across two real sites (allowlist and ExcludedSiteIDs denylist, including exclusion overriding inclusion), update rename collision, soft-deleted name reuse, cross-org isolation for all verbs, site-cascade soft-delete, stale-site-authorization fail-closed on Update and Delete, move-out-of-a-deleted-site fail-closed (the DeleteSite race window), audit-event emission (incl. driver_config exclusion and no-event-on-failure), validation matrix (kind, fan count, unknown driver, rejected endpoints, unit-ID bounds).
• Unit tests: registry resolution/duplicate panic/delegation; Modbus config validation including adversarial endpoint vectors (IPv4-mapped IPv6 public, unspecified, broadcast, multicast, CGNAT, loopback, link-local/IMDS — all rejected) and a regression pin that rejected endpoints never appear in error text; SiteScopeFor shapes (org-wide/denylist, allowlist, zero-permission narrowing, nil receiver); SiteScopeForPermission (allowlist/denylist projection, internal-actor short-circuit, unknown-actor and missing-permissions fail-closed, unauthenticated).
• Handler tests: per-RPC auth gates, site-narrowed callers denied on Get/Update/Delete, cross-site create denied, device move requiring manage on both sites, List filter composition (readable allowlist pushed to the store, narrowing denylist, request∩readable intersection, empty-readable short-circuit with no store call), driver_config redaction for read-only callers (redacted and included paths), ExpectedSiteID carried into the write, omitted-enabled defaulting on Create and presence pass-through on Update (with SQL-level preservation pinned by an integration test: disabled state survives a rename with enabled omitted), unauthenticated path, proto-domain translation round-trip, empty driver_config rejection.
• Config contract tests pin the five procedures in SensitiveBodyProcedures and SessionOnlyProcedures; RBAC contract test covers the new service via registeredServices; client tests pin the delete-dialog cascade summary (combined, fans-only, singular/plural). golangci-lint clean; buf lint clean; client typecheck/lint clean; just gen output committed.
• Not covered: SetState protocol I/O (stubbed by design, PR 4), client add/edit UI (PR 2), migration down-path execution.

Known Residuals

Accepted advisory findings from code review, deferred deliberately:

• P3 modbustcp.ParseConfig — unknown JSON keys in driver_config are persisted and echoed verbatim (within the 8KB cap); clients must treat the blob as untrusted when rendering.
• P3 store errors interpolate raw DB error text into Internal messages — matches the existing repo-wide sqlstore convention.
• The RFC1918/ULA guard is save-time only; dial-time per-site allowlist authorization is an explicit precondition of the PR 4 write path (see decisions above).

Resolved during the review rounds (each pinned by tests): site-scoped RBAC on all five RPCs, DeleteSite re-checked against the target site so narrowing can't be bypassed via the cascade, unreadable devices masked as NotFound on Get/Update/Delete so IDs can't be enumerated across site scopes, the service exposed via gRPC reflection alongside its siblings, driver_config redaction for site:read callers, sensitive-body log suppression, session-only enforcement on all five procedures, stale-site fail-closed write predicates, source-site lock on device moves (serializes against DeleteSite's cascade), fail-closed endpoint guard, validation and JSON-decode errors that never echo submitted config values (endpoint, unit_id, register_address, port, write_mode), register_address presence-tracked so omitted/null cannot silently mean register 0, activity-log audit trail, cascade count in DeleteSiteResponse/ListSites/delete dialog and through the client deleteSite hook, trimmed driver_type persistence, omitted enabled defaulting to true on Create and preserving the current value on Update, readable-site SQL filtering on List (no whole-org fetch).

Post-Deploy Monitoring & Validation

• Runtime impact: near zero — a new empty table, one new Connect service, one added statement inside the existing DeleteSite transaction, and one added count subquery in ListSites. No device commands can be issued (write path not implemented).
• Log queries: watch fleetd startup for migration 000122 applying cleanly; rg 'infrastructure_device' in server error logs for unexpected query failures.
• Healthy signal: InfrastructureService RPCs return expected codes (PermissionDenied for under-privileged callers, InvalidArgument for bad configs); DeleteSite and ListSites latency unchanged.
• Failure signal / rollback: migration failure at deploy blocks startup — down migration 000122 is provided; the service is additive, so reverting the deploy is safe with no data-loss risk (table would be dropped by the down migration only if explicitly run).
• Validation window / owner: first deploy cycle; owner: @rongxin.

Refs #723

[github-actions]

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

• Reviewed pull request diff only (cfdc535febc111ab1bd313226d3d6a405ed19042...38ad2bfea70f69ef7aafd2eb553ebc35cc767e8b, exact PR three-dot diff)
• Model: gpt-5.5

💡 Click "edited" above to see previous reviews for this PR.

---

Review Summary

Overall Risk: NONE

Findings

No findings.

Notes

Reviewed .git/codex-review.diff as the authoritative scope. No concrete security, correctness, reliability, protobuf compatibility, or cryptostealing/pool-hijack issues were identified in the latest changes.

---

_{Generated by Codex Security Review |
Triggered by: @rongxin-liu |
Review workflow run}

[Copilot]

Pull request overview

This PR introduces the server-side “infrastructure device” foundation (facility fans / fan groups) as a persisted domain concept, with a protocol-agnostic driver seam (registry + adapter validation) and a permission-gated Connect CRUD API. It’s PR 1/5 in the facility fan curtailment series and is intended to be operationally inert (no Modbus write path yet), while establishing the DB/proto/service surface area for subsequent PRs.

Changes:

• Adds infrastructure_device persistence (migration + sqlc queries + store + domain models) and wires it into fleetd.
• Implements a driver adapter registry plus a modbus_tcp adapter focused on config parsing/validation (write path stubbed).
• Adds Connect RPC CRUD handlers and RBAC procedure permission mapping; extends DeleteSite cascade to soft-delete infrastructure devices.

Reviewed changes

Copilot reviewed 27 out of 36 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
server/sqlc/queries/site.sql	Adds sqlc query to soft-delete infrastructure devices during site delete cascade
server/sqlc/queries/infrastructure_device.sql	New sqlc CRUD queries for infrastructure_device
server/migrations/000120_create_infrastructure_device_table.up.sql	Creates infrastructure_device table, constraints, indexes, updated_at trigger
server/migrations/000120_create_infrastructure_device_table.down.sql	Drops trigger/indexes/table for rollback
server/internal/handlers/sites/handler_test.go	Updates DeleteSite cascade expectations to include infra-device soft delete
server/internal/handlers/middleware/rpc_permissions.go	Registers InfrastructureService RPC permissions (site:read/site:manage)
server/internal/handlers/middleware/rpc_permissions_test.go	Adds InfrastructureService to RBAC contract coverage
server/internal/handlers/infrastructure/translate.go	Translates proto ↔ domain models for InfrastructureService
server/internal/handlers/infrastructure/handler.go	New Connect handler implementing InfrastructureService CRUD
server/internal/handlers/infrastructure/handler_test.go	Auth gating + translation tests for InfrastructureService handler
server/internal/domain/stores/sqlstores/site.go	Implements SoftDeleteInfrastructureDevicesBySite in SQLSiteStore
server/internal/domain/stores/sqlstores/infrastructure_device.go	New SQL store implementation for infrastructure devices
server/internal/domain/stores/interfaces/site.go	Extends SiteStore interface with infra-device cascade method
server/internal/domain/stores/interfaces/mocks/mock_site_store.go	Generated mock updated for new SiteStore method
server/internal/domain/stores/interfaces/mocks/mock_infrastructure_device_store.go	Generated mock for InfrastructureDeviceStore (generated)
server/internal/domain/stores/interfaces/infrastructure_device.go	New InfrastructureDeviceStore interface (+ mockgen directive)
server/internal/domain/sites/service.go	Extends DeleteSite cascade to soft-delete infrastructure devices
server/internal/domain/sites/service_test.go	Updates DeleteSite transaction cascade test expectations
server/internal/domain/sites/models/models.go	Adds DeletedInfrastructureDeviceCount to DeleteSiteResult
server/internal/domain/infrastructure/service.go	New domain service for infrastructure-device CRUD + normalization/validation
server/internal/domain/infrastructure/service_integration_test.go	Postgres integration tests for infra-device CRUD/validation/cascade
server/internal/domain/infrastructure/models/models.go	Domain model + params/filter types for infrastructure devices
server/internal/domain/infrastructure/driver/modbustcp/modbustcp.go	Modbus TCP adapter config schema + validation; SetState stub
server/internal/domain/infrastructure/driver/modbustcp/modbustcp_test.go	Unit tests for Modbus config validation + SetState stub behavior
server/internal/domain/infrastructure/driver/driver.go	Driver Controller interface + registry implementation
server/internal/domain/infrastructure/driver/driver_test.go	Unit tests for registry resolution/validation delegation
server/cmd/fleetd/main.go	Wires infrastructure store/service and mounts InfrastructureService handler
proto/infrastructure/v1/infrastructure.proto	Defines InfrastructureService + messages (opaque driver_config string)
docs/plans/2026-07-09-facility-fan-curtailment-integration-plan.md	Plan doc for the 5-PR delivery stack
server/generated/sqlc/site.sql.go	Generated sqlc output (generated — skip)
server/generated/sqlc/models.go	Generated sqlc model for InfrastructureDevice (generated — skip)
server/generated/sqlc/infrastructure_device.sql.go	Generated sqlc bindings (generated — skip)
server/generated/sqlc/db.go	Generated sqlc prepared statements wiring (generated — skip)
server/generated/grpc/infrastructure/v1/infrastructurev1connect/infrastructure.connect.go	Generated Connect stubs (generated — skip)
server/generated/grpc/infrastructure/v1/infrastructure.pb.go	Generated protobuf Go types (generated — skip)
client/src/protoFleet/api/generated/infrastructure/v1/infrastructure_pb.ts	Generated TS protobuf types (generated — skip)

Files not reviewed (2)

• server/internal/domain/stores/interfaces/mocks/mock_infrastructure_device_store.go: Generated file
• server/internal/domain/stores/interfaces/mocks/mock_site_store.go: Generated file

[rongxin-liu]

Both Codex security findings are addressed in c799323:

[MEDIUM] Site-scoped RBAC bypass — all five RPCs now authorize against the device's site via ResourceContext{SiteID} instead of the bare org context:

• Create checks site:manage on the request's site.
• Get/Update/Delete resolve the device under org scope, then authorize its current site (same resolve-then-authorize pattern and accepted same-org existence leak as buildings.GetBuildingStats); Update additionally checks the target site when the device moves, so a manager of only one side can't pull a device across.
• List filters results to sites the caller can read, so site-narrowed operators see their sites' devices and nothing else.
• New site-scoped RBAC tests: narrowed-away caller denied on Get/Update/Delete, cross-site create denied, move requiring manage on both sites, and list filtering (handler_test.go).

[MEDIUM] driver_config in debug logs — all five InfrastructureService procedures are now in SensitiveBodyProcedures (same rationale as the fleet-topology entries), so request/response bodies carrying the OT endpoint map are suppressed even at debug level; pinned by TestInfrastructureProceduresAreSensitiveBody.

The note about the endpoint guard admitting loopback/link-local (incl. IMDS) targets remains deliberate for v1 — it's pinned by an explicit test and flagged in the PR description to revisit when the write path lands (PR 4).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c799323a2a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[rongxin-liu]

Both new Codex security findings are addressed in 523891b:

[MEDIUM] Mutations authorize a stale site — Update and Delete now predicate the SQL write on the site the caller was authorized against. The handler resolves the device, authorizes its current site (and, on a move, the target site), then passes that current site into the mutation as expected_site_id, which is now part of the UPDATE/soft-delete WHERE clause. A concurrent move between the authorization read and the write matches 0 rows → NotFound, so a caller cannot overwrite or delete a device that has since moved to a site they don't manage. ExpectedSiteID is threaded through UpdateParams, the store interface, and the delete signature; TestService_StaleSiteAuthorizationFailsClosed_DatabaseIntegration proves both paths fail closed after a simulated move, and a handler test asserts the authorized site is carried into the write.

[MEDIUM] Endpoint validation permits server-local/metadata targets — the guard now fails closed: only private RFC1918 / IPv6 ULA addresses are accepted. Loopback, link-local (incl. 169.254.169.254 IMDS), multicast, broadcast, unspecified, and public addresses are all rejected, removing the SSRF/local-pivot surface before the write path is wired. Tests updated to assert the rejections; the plan doc records that any non-RFC1918 control endpoint would be a future explicit per-site allowlist decision rather than a blanket allowance.

All touched packages pass (including the Postgres integration suite) and golangci-lint is clean.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 523891bc9b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b6e9850da5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[rongxin-liu]

Both findings from the latest security review are addressed in 79b3a72:

[MEDIUM] Read APIs expose full driver configuration to site-read callers — driver_config is now redacted from read responses unless the caller holds site:manage for the device's site. Get and List evaluate this per device (a caller managing site A but only reading site B sees A's config and not B's); Create/Update echo the config their existing site:manage gate already covers. Display fields (name, kind, driver type, site) remain visible at site:read. TestHandler_DriverConfigRedactedForReadOnlyCallers pins both the redacted and included paths, and the proto field comment documents the contract. Encrypted storage for future credential-bearing configs remains the recorded adapter-contract seam in the plan's FYI list (no credentials exist in any v1 config).

[MEDIUM] Private-IP validation too broad for future raw Modbus writes — agreed that RFC1918 is a save-time bound, not dial-time authorization. The full fix (a per-site commissioned control-subnet allowlist plus server-infrastructure CIDR rejection) requires per-site commissioning data that doesn't exist until the write path's commissioning checklist runs, so it lands with the write path itself — and is now recorded as an explicit SECURITY PRECONDITION doc comment on modbustcp.SetState and as a dial-time precondition section in the plan doc: writes stay disabled for a site until its allowlist is commissioned. The plan already gates OT-segment reachability through fleetnode-style site-local execution where the central server has no route.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a09254e800

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 64975ce31c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6e7687e708

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b5eea94c38

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".