feat: secure worker runtime with OIDC - #14
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6106e3fd3
ℹ️ 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".
| async get(@Param('jobId') jobId: string) { | ||
| const job = await this.store.get(jobId); | ||
| if (!job) { |
There was a problem hiding this comment.
When a user with workbench.read knows or obtains another tenant's job id, this method returns the WorkerJob without comparing job.tenant_id to req.tenantId, even though job payloads/results can contain tenant-specific payment or compliance data. The create path now forces jobs onto the caller's tenant, but the read path still allows cross-tenant disclosure; take @Req() here and reject jobs whose tenant_id does not match the authenticated tenant.
Useful? React with 👍 / 👎.
| @Get('metrics') | ||
| @Public() | ||
| @RequirePermissions('observability.read') | ||
| @Header('Content-Type', 'text/plain; version=0.0.4') |
There was a problem hiding this comment.
With @Public() set here, AccessTokenGuard and PermissionsGuard both return before checking the bearer token or observability.read, so any unauthenticated request can scrape /api/metrics despite the permission annotation. If Prometheus metrics are supposed to be protected by the new OIDC/permission model, this decorator makes the permission check unreachable.
Useful? React with 👍 / 👎.
|
|
||
| export class CreateJobV1Dto { | ||
| @IsOptional() @IsString() @Matches(/^[a-z][a-z0-9_-]{1,63}$/) queue?: string; | ||
| @IsIn(JOB_TYPES) type!: JobType; |
There was a problem hiding this comment.
Because this DTO accepts the full JOB_TYPES list, any caller with only workbench.jobs.write can submit internal worker types such as LEDGER_CORE_EVENT/FENGINE_EVENT and have the worker call Ledger Core with its internal.worker service token. The new OpenAPI contract limits /api/jobs to payment job types, so this should validate against a public allowlist instead of every worker type.
Useful? React with 👍 / 👎.
| redisUrl: process.env.REDIS_URL || 'redis://localhost:16379', | ||
| databaseUrl: process.env.DATABASE_URL || 'postgresql://mavula:mavula_dev@localhost:15432/mavula?schema=public', | ||
| databaseUrl, | ||
| legacyConnectorsDatabaseUrl: env('LEGACY_CONNECTORS_DATABASE_URL', databaseUrl)!, |
There was a problem hiding this comment.
When LEGACY_CONNECTORS_DATABASE_URL points at a separate Postgres instance, legacy batch reads/writes use that URL but /api/status still probes only DATABASE_URL, so an outage of the legacy batch store is reported as healthy until compliance endpoints start failing. Either include this configured database in platform status or avoid allowing it to diverge from the checked database.
Useful? React with 👍 / 👎.
| catch (error) { | ||
| if (error instanceof BadRequestException) throw error; | ||
| if (error instanceof LegacyBatchConflictError || error instanceof LegacyBatchStateError) throw new ConflictException(error.message); | ||
| throw new BadRequestException((error as Error).message); |
There was a problem hiding this comment.
This catch-all converts every unexpected failure during export/import creation or delivery into a 400 response, so Redis enqueue failures or Postgres/storage errors are reported to clients as bad requests rather than server/dependency failures. Keep the BadRequestException and known legacy conflict/state mappings, but let unknown errors propagate as 5xx so operators and clients can distinguish invalid input from infrastructure failures.
Useful? React with 👍 / 👎.
1 participant
Summary
internal.workerValidation
pnpm buildpnpm test:all(20 unit tests and 3 e2e tests)pnpm guardian:checkCoordination
Depends on the identity-access token endpoint and the coordinated ledger-core resource-server PR.