fix: block SQL injection in role attributes#436
Conversation
Validate database_users[].attributes against a fixed keyword allowlist in postgres.CreateUserRole instead of interpolating them unescaped into ALTER ROLE. Also bump containerd, go-git, go-billy, and the same pgx/otel/x-crypto/x-net/chi/Go-toolchain set from PLAT-686 to close the remaining Codacy findings. Risk-accept the unfixed docker/docker CVEs in .trivy/pgedge-control-plane.trivyignore.yaml. PLAT-685
📝 WalkthroughWalkthroughThe change adds Docker command and PostgreSQL role-attribute validation, refactors API, database, orchestration, monitoring, migration, and workflow paths into helpers, updates Go dependencies and toolchains, pins build inputs, refreshes license records, and expands Trivy suppressions. ChangesSecurity and API validation
Configuration and lifecycle refactoring
Orchestration, migration, monitoring, and workflows
Dependency and build refresh
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/Dockerfile (1)
3-6: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
--no-cache-dirto reduce the image size.Consider adding the
--no-cache-dirflag to thepip installcommand. This preventspipfrom caching the downloaded packages, which is generally unnecessary in Docker containers and helps keep the final image size smaller.♻️ Proposed refactor
-RUN pip install \ +RUN pip install --no-cache-dir \ pymdown-extensions==11.0.1 \ mkdocs-redoc-tag==0.2.0 \ mkdocs-github-admonitions-plugin==0.1.1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/Dockerfile` around lines 3 - 6, Update the pip install command in the Dockerfile to include the --no-cache-dir option while preserving the existing package versions and multiline installation structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/Dockerfile`:
- Around line 3-6: Update the pip install command in the Dockerfile to include
the --no-cache-dir option while preserving the existing package versions and
multiline installation structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49658f9e-8be4-4fe5-bc88-f991d5845294
📒 Files selected for processing (5)
NOTICE.txtdocker/control-plane-ci/Dockerfiledocs/Dockerfileserver/internal/docker/docker.goserver/internal/docker/docker_test.go
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/internal/orchestrator/swarm/orchestrator.go (1)
302-310: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOnly reuse
ResolvedImagewhen every image-selection input is unchanged.A same-version service-type change reuses the old service’s image. Adding a user
Imageoverride also leaves bothImageandResolvedImageset, violating their mutual-exclusion contract.Proposed fix
func serviceVersionUnchanged(old, new *database.ServiceInstanceSpec) bool { - return old != nil && old.ServiceSpec != nil && new.ServiceSpec != nil && - old.ServiceSpec.Version == new.ServiceSpec.Version + if old == nil || old.ServiceSpec == nil || new.ServiceSpec == nil { + return false + } + if old.ServiceSpec.ServiceType != new.ServiceSpec.ServiceType || + old.ServiceSpec.Version != new.ServiceSpec.Version { + return false + } + opts := new.ServiceSpec.OrchestratorOpts + return opts == nil || opts.Swarm == nil || opts.Swarm.Image == "" }Also applies to: 316-337
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/internal/orchestrator/swarm/orchestrator.go` around lines 302 - 310, Update ReconcileServiceInstanceSpec and its serviceVersionUnchanged decision so the old ResolvedImage is carried forward only when all image-selection inputs, including service type and user Image override, are unchanged. When any such input changes, clear the new Swarm.ResolvedImage so resolution uses the current manifest or override, preserving the mutual exclusion between Image and ResolvedImage.
🧹 Nitpick comments (1)
server/internal/api/apiv1/validate.go (1)
190-195: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify the error message for invalid source node chaining.
The error message "source node must refer to an existing node" is slightly misleading here, as the node does exist in the spec (it passed the
seenNodeNamescheck above). The actual failure is that the referenced node is also being provisioned (it declares its ownsource_node). Updating the message will help users diagnose the validation failure faster.💡 Proposed fix
if newNodesWithSource.Has(src) { errs = append(errs, validation.NewError( - errors.New("source node must refer to an existing node"), + errors.New("source node cannot refer to a node that is also being provisioned from a source"), srcPath, )) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/internal/api/apiv1/validate.go` around lines 190 - 195, Update the validation error created in the newNodesWithSource check to state that source nodes cannot themselves define a source_node or be provisioned from another source, rather than claiming the referenced node does not exist. Keep the existing validation condition and srcPath unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/internal/api/apiv1/pre_init_handlers.go`:
- Around line 106-108: In the join-credentials handling around
decodeJoinCredentials, validate opts.Credentials for nil before passing it to
the decoder. Return an appropriate API error when credentials are absent, and
preserve the existing decode error path for non-nil credentials.
In `@server/internal/config/manager.go`:
- Around line 107-126: Correct the duplicated error messages in the
configuration merge and unmarshal paths. Update each fmt.Errorf call around
combinedK.Merge and generatedK/combinedK.Unmarshal to identify the specific
operation and configuration layer that failed, while preserving the existing
wrapped errors and return behavior.
In `@server/internal/monitor/instance_monitor.go`:
- Around line 90-93: Update checkStatus and the available-state transition
around updateInstanceStatus so failures from UpdateInstanceState are returned
instead of discarded. Ensure checkStatus does not publish a healthy status when
the instance state update fails, while preserving the existing status update
flow on success.
In `@server/internal/orchestrator/swarm/mcp_config.go`:
- Around line 246-273: In server/internal/orchestrator/swarm/mcp_config.go lines
246-273, update buildMCPKBConfig to assume validated input: remove the defensive
nil checks and related comment, and change it to return only *mcpKBConfig. In
server/internal/orchestrator/swarm/mcp_config.go lines 114-117, update the
caller to assign the direct result without handling an error or retaining the
removed error variable.
In `@server/internal/orchestrator/swarm/service_spec.go`:
- Around line 155-158: Update the extra-label merge in the swarm service
specification so entries from swarmOpts.ExtraLabels cannot overwrite the
control-plane identity labels pgedge.component or pgedge.service.instance.id.
Preserve all other user-provided labels, and ensure the control-plane values
remain authoritative for ServiceInspectByLabels reconciliation.
In `@server/internal/workflows/backend/etcd/etcd.go`:
- Around line 661-668: Update the transaction error handling around the
`b.store.Txn(...).Commit(ctx)` call so only the specific transaction-conflict
error returns `(nil, nil)` as a lock race; propagate all other etcd or
serialization errors to the caller instead of suppressing them.
- Around line 647-652: Update the lock reassignment branch in the activity-lock
handling around lock.CanBeReassignedTo so the Update operation uses
b.options.ActivityLockTimeout instead of b.options.WorkflowLockTimeout. Keep the
existing reassignment and update flow unchanged.
In `@server/internal/workflows/delete_database.go`:
- Around line 127-130: Update deleteDatabaseResources so the error returned by
operations.UpdateDatabase is passed through handleError before returning,
instead of being returned directly. Preserve the existing planning error context
while ensuring the caller receives handleError’s result and the database/task
failure state is recorded.
In `@server/internal/workflows/failover.go`:
- Around line 79-82: Update the skipped-candidate branch in the failover
workflow to propagate errors returned by updateFailoverTask instead of
discarding them. Preserve the existing success response when task completion
succeeds, but return the completion error so the workflow cannot report success
while the task remains running.
---
Outside diff comments:
In `@server/internal/orchestrator/swarm/orchestrator.go`:
- Around line 302-310: Update ReconcileServiceInstanceSpec and its
serviceVersionUnchanged decision so the old ResolvedImage is carried forward
only when all image-selection inputs, including service type and user Image
override, are unchanged. When any such input changes, clear the new
Swarm.ResolvedImage so resolution uses the current manifest or override,
preserving the mutual exclusion between Image and ResolvedImage.
---
Nitpick comments:
In `@server/internal/api/apiv1/validate.go`:
- Around line 190-195: Update the validation error created in the
newNodesWithSource check to state that source nodes cannot themselves define a
source_node or be provisioned from another source, rather than claiming the
referenced node does not exist. Keep the existing validation condition and
srcPath unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 885d23dc-d6a3-4da6-981d-98d183d4cb8f
📒 Files selected for processing (49)
api/apiv1/design/database.goapi/apiv1/design/task.godocker/control-plane/Dockerfiledocs/Dockerfileserver/internal/api/apiv1/post_init_handlers.goserver/internal/api/apiv1/pre_init_handlers.goserver/internal/api/apiv1/validate.goserver/internal/config/config.goserver/internal/config/manager.goserver/internal/database/instance_resource.goserver/internal/database/operations/end.goserver/internal/database/operations/populate_nodes.goserver/internal/database/operations/update_database.goserver/internal/database/postgres_database.goserver/internal/database/reconcile_versions.goserver/internal/database/replication_slot_advance_from_cts_resource.goserver/internal/database/script.goserver/internal/database/service.goserver/internal/database/spec.goserver/internal/database/wait_for_sync_event_resource.goserver/internal/etcd/provide.goserver/internal/etcd/rbac.goserver/internal/ipam/service.goserver/internal/migrate/migrations/add_task_scope.goserver/internal/monitor/instance_monitor.goserver/internal/monitor/provide.goserver/internal/monitor/service_instance_monitor.goserver/internal/orchestrator/common/patroni_config.goserver/internal/orchestrator/common/postgres_certs.goserver/internal/orchestrator/swarm/mcp_config.goserver/internal/orchestrator/swarm/orchestrator.goserver/internal/orchestrator/swarm/pgbackrest_restore.goserver/internal/orchestrator/swarm/rag_config.goserver/internal/orchestrator/swarm/service_instance.goserver/internal/orchestrator/swarm/service_spec.goserver/internal/orchestrator/systemd/pgbackrest_restore.goserver/internal/orchestrator/systemd/provide.goserver/internal/orchestrator/systemd/unit_options.goserver/internal/pgbackrest/config.goserver/internal/postgres/hba/parse.goserver/internal/resource/migrations/1_0_0.goserver/internal/resource/migrations/1_2_0.goserver/internal/resource/migrations/schematool/identifier.goserver/internal/resource/topo.goserver/internal/workflows/activities/check_cluster_health.goserver/internal/workflows/activities/select_candidate_instance.goserver/internal/workflows/backend/etcd/etcd.goserver/internal/workflows/delete_database.goserver/internal/workflows/failover.go
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
server/internal/docker/docker.go (1)
81-82: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winNormalize the executable path before checking the shell denylist.
Equivalent paths such as
/usr/bin/sh,/usr/local/bin/bash, and./dashbypass the exact-string set and still enable-ccommand interpretation. Comparepath.Base(command[0])against interpreter names instead.Proposed fix
- if shellInterpreters.Has(command[0]) { + if shellInterpreters.Has(path.Base(command[0])) {Add the standard-library
pathimport.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/internal/docker/docker.go` around lines 81 - 82, Update the shell interpreter check in the command validation logic to compare path.Base(command[0]) with shellInterpreters instead of the raw executable path. Add the standard-library path import and preserve the existing ErrShellInterpreterNotAllowed error behavior.server/internal/database/service.go (1)
771-775: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRestore allocated fields during rollback.
The rollback releases newly allocated ports but leaves
spec.Portandspec.PatroniPortpopulated. A retry can therefore skip allocation and persist a port that has already returned to the pool.Proposed fix
func (s *Service) allocateInstanceSpecPorts(ctx context.Context, spec *InstanceSpec) (func(error) error, error) { + originalPort := spec.Port + originalPatroniPort := spec.PatroniPort var allocated []int rollback := func(cause error) error { rollbackCtx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() - return errors.Join(cause, s.portsSvc.ReleasePort(rollbackCtx, spec.HostID, allocated...)) + releaseErr := s.portsSvc.ReleasePort(rollbackCtx, spec.HostID, allocated...) + spec.Port = originalPort + spec.PatroniPort = originalPatroniPort + return errors.Join(cause, releaseErr) }Based on learnings, reconciliation retries must remain idempotent and non-destructive.
Also applies to: 778-794
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/internal/database/service.go` around lines 771 - 775, Update the rollback closure to restore the allocation fields after releasing the ports: clear spec.Port and spec.PatroniPort before returning, while preserving the existing joined-error behavior and timeout cleanup. Ensure retries re-enter port allocation rather than reusing released ports.Source: Learnings
server/internal/workflows/backend/etcd/etcd.go (1)
591-639: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStop routing events to an already-existing sub-workflow.
When the child exists,
appendSubWorkflowStartOpsenqueuesSubWorkflowFailed, but Line 599 still sends the original start event to that existing execution. Return a routing flag and skip the event loop for this collision.Proposed control-flow fix
-ops, err = b.appendSubWorkflowStartOps(...) +var routeEvents bool +ops, routeEvents, err = b.appendSubWorkflowStartOps(...) if err != nil { return nil, err } +if !routeEvents { + continue +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/internal/workflows/backend/etcd/etcd.go` around lines 591 - 639, Update appendSubWorkflowStartOps and its caller to return a routing flag indicating whether the child already exists; when that collision is detected, enqueue SubWorkflowFailed but mark the event batch as not routable. In the caller’s event-processing flow, skip the loop that appends PendingEvent records for the original start event when the flag is false, while preserving normal routing for newly created sub-workflows.server/internal/workflows/service.go (1)
503-516: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not orphan a running remove-host workflow when task linkage fails.
CreateWorkflowInstancemay succeed beforeUpdateTaskfails.abortTasksonly changes task metadata, so host removal can continue while the caller receives an error and potentially retries.Persist a deterministic workflow instance ID before dispatch, or cancel the created workflow when linkage cannot be stored.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/internal/workflows/service.go` around lines 503 - 516, The remove-host flow around CreateWorkflowInstance and UpdateTask must not leave a successfully created workflow running when task linkage fails. Persist a deterministic workflow instance ID before dispatch where supported, or cancel the created workflow instance before returning the UpdateTask error; retain abortTasks and the existing error propagation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/internal/api/apiv1/post_init_handlers.go`:
- Line 510: Normalize the optional req.Options before calling
startBackupWorkflow in the relevant initialization handlers, including the paths
around the calls at lines 510 and 569-572. Ensure omitted options remain safely
represented as nil or are replaced with the established default without allowing
startBackupWorkflow to dereference a nil value.
- Around line 597-601: Make the switchover creation flow around
checkNoActiveSwitchover and startSwitchoverWorkflow atomic so concurrent
requests cannot create multiple switchovers. Use a transaction-scoped lock or
deterministic unique key/workflow instance ID enforced by the persistence layer,
and ensure conflicting creations are rejected while preserving the existing
successful workflow path.
In `@server/internal/database/mcp_service_config.go`:
- Around line 202-216: Update parseLLMFields so ollamaURL is parsed whenever
either the LLM configuration uses Ollama or embeddingProvider is Ollama,
including the llm_enabled path. Preserve the existing parseLLMFieldsEnabled
results while supplying the shared ollama_url for OpenAI-LLM/Ollama-embedding
configurations.
In `@server/internal/etcd/embedded.go`:
- Around line 667-675: Update finalizeClientModeConfig to persist the
client-mode generated configuration before calling os.RemoveAll(e.etcdDir()). If
cfg.UpdateGeneratedConfig fails, return the existing wrapped error without
deleting the embedded data; only remove the data directory after the
configuration update succeeds, preserving the existing removal error handling.
In `@server/internal/orchestrator/swarm/orchestrator.go`:
- Around line 1210-1241: Populate the InstanceID field in every
database.ValidationResult constructed by the validation paths around
parseImageTag and the additional reported ranges, using the current instance’s
identifier from cur. Preserve the existing validation status, node, host, and
error contents while ensuring all returned results carry the corresponding
InstanceID.
In `@server/internal/postgres/create_db.go`:
- Around line 184-210: Update subInterfaceSwapStatements to generate a
collision-resistant interfaceName suffix instead of using time.Now().Unix()
second resolution. Preserve the existing providerName-based naming format and
ensure concurrent updates or immediate retries produce distinct names before
node_add_interface executes.
In `@server/internal/workflows/switchover.go`:
- Around line 183-186: Update the candidateID == leaderInstanceID skip path to
capture and propagate errors returned by w.updateSwitchoverTask when marking the
task complete. Return the existing success result only when the completion
update succeeds; otherwise return the update error instead of ignoring it.
---
Outside diff comments:
In `@server/internal/database/service.go`:
- Around line 771-775: Update the rollback closure to restore the allocation
fields after releasing the ports: clear spec.Port and spec.PatroniPort before
returning, while preserving the existing joined-error behavior and timeout
cleanup. Ensure retries re-enter port allocation rather than reusing released
ports.
In `@server/internal/docker/docker.go`:
- Around line 81-82: Update the shell interpreter check in the command
validation logic to compare path.Base(command[0]) with shellInterpreters instead
of the raw executable path. Add the standard-library path import and preserve
the existing ErrShellInterpreterNotAllowed error behavior.
In `@server/internal/workflows/backend/etcd/etcd.go`:
- Around line 591-639: Update appendSubWorkflowStartOps and its caller to return
a routing flag indicating whether the child already exists; when that collision
is detected, enqueue SubWorkflowFailed but mark the event batch as not routable.
In the caller’s event-processing flow, skip the loop that appends PendingEvent
records for the original start event when the flag is false, while preserving
normal routing for newly created sub-workflows.
In `@server/internal/workflows/service.go`:
- Around line 503-516: The remove-host flow around CreateWorkflowInstance and
UpdateTask must not leave a successfully created workflow running when task
linkage fails. Persist a deterministic workflow instance ID before dispatch
where supported, or cancel the created workflow instance before returning the
UpdateTask error; retain abortTasks and the existing error propagation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a0006f6b-2b21-430a-8446-c3d0a4b50b59
📒 Files selected for processing (42)
api/apiv1/design/database.goapi/apiv1/design/task.gomqtt/endpoint.goserver/cmd/root.goserver/internal/api/apiv1/convert.goserver/internal/api/apiv1/post_init_handlers.goserver/internal/api/apiv1/validate.goserver/internal/app/app.goserver/internal/certificates/service.goserver/internal/config/config.goserver/internal/database/mcp_service_config.goserver/internal/database/operations/update_database.goserver/internal/database/postgrest_service_config.goserver/internal/database/service.goserver/internal/docker/docker.goserver/internal/etcd/embedded.goserver/internal/orchestrator/common/etcd_creds.goserver/internal/orchestrator/common/patroni_config_generator.goserver/internal/orchestrator/swarm/orchestrator.goserver/internal/orchestrator/swarm/rag_config.goserver/internal/orchestrator/swarm/service_instance.goserver/internal/orchestrator/swarm/spec.goserver/internal/orchestrator/systemd/orchestrator.goserver/internal/postgres/create_db.goserver/internal/resource/migrations/1_0_0.goserver/internal/resource/migrations/1_2_0.goserver/internal/resource/migrations/schemas/v1_2_0/swarm.goserver/internal/resource/migrations/schematool/inliner.goserver/internal/resource/migrations/schematool/output.goserver/internal/resource/migrations/schematool/parser.goserver/internal/resource/state.goserver/internal/storage/txn.goserver/internal/workflows/activities/apply_event.goserver/internal/workflows/backend/etcd/etcd.goserver/internal/workflows/common.goserver/internal/workflows/create_pgbackrest_backup.goserver/internal/workflows/pgbackrest_restore.goserver/internal/workflows/plan_update.goserver/internal/workflows/refresh_current_state.goserver/internal/workflows/service.goserver/internal/workflows/switchover.goserver/internal/workflows/update_database.go
c7043e7 to
05f074d
Compare
Summary
Fixes a real SQL injection bug in role creation, cleans up a false-positive finding on the same scan, risk-accepts a couple of CVEs that don't have fixes yet, and fixes the licenses-ci CI failure.
Changes
SQL injection fix.
CreateUserRoleinroles.gowas buildingALTER ROLE ... WITH <attr>by pasting each attribute string straight into the SQL. A bad attribute like"LOGIN; DROP TABLE users; --"would just run. Added an allowlist of the actual valid role attribute keywords (SUPERUSER, CREATEDB, LOGIN, etc. plus their NO* forms) and reject anything else before it gets near a SQL statement. Left out the attribute forms that take a literal value (CONNECTION LIMIT, VALID UNTIL, PASSWORD) since those can't be safely allowlisted this way and password already has its own field. Added tests in roles_test.go. Also tried this for real against a running dev cluster: sent the malicious attribute through the actual API, and it got rejected with "unsupported role attribute" before touching Postgres.False positive cleanup. The scanner also flagged
PostgresContainerExecin utils.go as SQL injection, but it's a Docker exec call, not SQL. Checked every place that calls it and confirmed the command is always built from fixed binaries, never raw input, and Docker doesn't run it through a shell anyway. Added a comment explaining why so it doesn't keep getting flagged.Risk acceptances. Added two entries to the trivyignore file: the docker/docker CVEs that have no fix in the v27 line we're on (and we don't use the code paths they affect anyway), and the x/crypto/openpgp advisory, which doesn't apply since we don't import that package.
Fixed the licenses-ci failure. NOTICE.txt had gotten out of sync with go.mod, and separately go-licenses has a bug where it generates broken LICENSE links for some opentelemetry-go packages. Fixed the link bug in the Makefile so it's part of regeneration going forward instead of a one-off patch, then regenerated NOTICE.txt properly in a container that matches CI's Go version (couldn't do it locally on macOS, go-licenses crashes there for an unrelated reason). Ran it twice to make sure it's stable.
Testing
go build, go vet, go test all pass. Tested the injection fix live against a dev cluster. Regenerated NOTICE.txt in a matching CI container twice, same output both times.
Checklist
Notes for reviewers
The NOTICE.txt/Makefile fix also applies to the PLAT-686 branch, which hit the same CI failure for the same reason.