Resolve benchmark workflows for seed#2006
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a benchmark workflow “catalog resolver” to map provider/workflow IDs to on-disk JSON workflow definitions, primarily to support benchmark seeding.
Changes:
- Introduces
catalog-resolverhelpers to resolve workflow JSON file paths (including seed-specific orchestration ordering). - Adds provider manifest mapping orchestrator/cleanup workflow IDs.
- Copies
workflows-catalog/**/*into the server-api build output and adds unit tests for the resolver.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/server/api/src/app/benchmark/catalog-resolver.ts | New resolver that builds file paths for workflow catalog JSON files. |
| packages/server/api/src/app/benchmark/catalog-manifests.ts | Adds provider→manifest mapping for orchestrator/cleanup workflow IDs. |
| packages/server/api/test/unit/benchmark/catalog-resolver.test.ts | Unit tests covering unsupported provider, missing files, empty inputs, and seed ordering. |
| packages/server/api/project.json | Ensures workflow catalog JSON assets are included in build output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ): ResolvedWorkflowPath[] { | ||
| const catalogDir = getCatalogDir(provider); | ||
| const result: ResolvedWorkflowPath[] = []; | ||
| for (const id of workflowIds) { |
There was a problem hiding this comment.
resolveWorkflowPaths builds paths from untrusted provider/id values using path.join(catalogDir, ${id}.json). If either contains path separators or .., this can resolve outside the workflows catalog directory (path traversal) and allow probing/reading arbitrary files when the returned filePath is later used. Validate provider via getCatalogManifest() inside resolveWorkflowPaths, and reject/normalize id values (e.g., forbid //\\ and .., or path.resolve + ensure the resolved path stays under catalogDir).
| ): ResolvedWorkflowPath[] { | |
| const catalogDir = getCatalogDir(provider); | |
| const result: ResolvedWorkflowPath[] = []; | |
| for (const id of workflowIds) { | |
| ): ResolvedWorkflowPath[] { | |
| // Validate provider against the known catalog manifests. | |
| getCatalogManifest(provider); | |
| const catalogDir = getCatalogDir(provider); | |
| const result: ResolvedWorkflowPath[] = []; | |
| for (const id of workflowIds) { | |
| // Reject workflow IDs that could lead to path traversal or invalid filenames. | |
| if (id.includes('/') || id.includes('\\') || id.includes('..')) { | |
| throwValidationError(`Invalid workflow id: ${id}`); | |
| } |
There was a problem hiding this comment.
Valid finding: the traversal concern is legit, the provider concern is less so, but still doesn't hurt to act on it.
|
| provider: string, | ||
| subWorkflowIds: string[], | ||
| ): ResolvedWorkflowPath[] { | ||
| if (subWorkflowIds.length === 0) { |
There was a problem hiding this comment.
I might move this validation later to another function when we have a proper service file for this endpoint. Keeping it here for now
Fixes OPS-3751.
Additional Notes
This PR adds a helper method to return paths for benchmark workflows from the catalog for a given provider and user chosen workflow ids.
The next step would be to import the flows using these paths.