Open
Conversation
Conflict resolutions: - inputs.py, output.py: kept HEAD's mthds_contents (list[str]) over dev's singular mthds_content
Greptile SummaryThis PR merges staging into main, introducing multi-file PLX content support (
Confidence Score: 4/5Two P1 issues should be addressed before merging to main: unprotected dry_run_pipeline and spoofable identity headers in no_auth. The P1 in validate.py causes silent 500 errors for valid PLX content that fails dry-run; the P1 in security.py is a real identity-spoofing vector when the server is misconfigured. Both are straightforward to fix. All remaining findings are P2. api/routes/pipelex/validate.py (unprotected dry_run_pipeline), api/security.py (spoofable identity headers in no_auth)
|
| Filename | Overview |
|---|---|
| api/security.py | Refactored to support three auth modes; the no_auth mode trusts forwarded identity headers from any caller, enabling impersonation when not behind an API Gateway. |
| api/routes/pipelex/validate.py | Extended validate endpoint with dry_run_pipeline and pipe_structures, but dry_run_pipeline is outside the error-handling block and the endpoint is missing response_model. |
| api/routes/pipelex/pipeline.py | Updated to support mthds_contents array; execute now returns JSONResponse for proper serialization. |
| api/routes/pipelex/agent/models.py | New endpoint to list available model presets; missing response_model on the route decorator. |
| api/routes/uploader.py | Now uses get_request_user dependency to set user_id from authenticated identity rather than a hardcoded placeholder. |
| Dockerfile | Adds cp of .pipelex config into /root/.pipelex to ensure pipelex global config is available at runtime. |
Sequence Diagram
sequenceDiagram
participant Client
participant FastAPI
participant AuthLayer
participant RouteHandler
participant PipelexCore
Client->>FastAPI: HTTP Request
FastAPI->>AuthLayer: get_auth_dependency() selects mode
alt no_auth mode
AuthLayer->>AuthLayer: Read forwarded identity headers
AuthLayer->>FastAPI: Set request.state.user if headers present
else jwt mode
AuthLayer->>AuthLayer: Decode and verify JWT token
AuthLayer->>FastAPI: Set request.state.user from JWT payload
else api_key mode
AuthLayer->>AuthLayer: Validate static API key
FastAPI->>FastAPI: request.state.user remains None
end
FastAPI->>RouteHandler: Dispatch to route
RouteHandler->>RouteHandler: get_request_user() returns RequestUser or None
RouteHandler->>PipelexCore: validate_bundle / dry_run_pipeline / assemble_bundle
PipelexCore-->>RouteHandler: Result
RouteHandler-->>Client: JSONResponse
Prompt To Fix All With AI
This is a comment left during a code review.
Path: api/routes/pipelex/validate.py
Line: 97
Comment:
**`dry_run_pipeline` called outside error-handling block**
`dry_run_pipeline` is called after the `try/except ValidateBundleError` block, so any exception it raises propagates as an unhandled HTTP 500 rather than returning a clean `{"success": false, ...}` JSON response — breaking the error contract established for `validate_bundle` just above.
```suggestion
try:
graph_spec, _ = await dry_run_pipeline(mthds_contents=mthds_contents)
except Exception as exc:
return JSONResponse(
content={
"success": False,
"mthds_contents": mthds_contents,
"message": str(exc),
}
)
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: api/security.py
Line: 154-165
Comment:
**Forwarded identity headers are spoofable without API Gateway enforcement**
`no_auth` unconditionally trusts the email and identity headers sent by the caller. If the server is reachable directly rather than via an API Gateway, any caller can inject arbitrary values and receive a fully-populated `RequestUser`. Downstream handlers that use the identity to scope operations (such as the uploader endpoint) will act on the spoofed identity. The docstring documents the API Gateway requirement but provides no runtime enforcement.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: api/routes/pipelex/validate.py
Line: 84-86
Comment:
**Potential `IndexError` if `blueprints` is empty**
If `_find_main_blueprint` returns `None` and `validate_bundle_result.blueprints` happens to be an empty list, `blueprints[0]` will raise an `IndexError`. Adding a guard here makes the error message explicit:
```suggestion
primary_blueprint = _find_main_blueprint(validate_bundle_result.blueprints)
if not primary_blueprint:
if not validate_bundle_result.blueprints:
return JSONResponse(
content={
"success": False,
"mthds_contents": mthds_contents,
"message": "No bundle blueprints found after validation",
}
)
primary_blueprint = validate_bundle_result.blueprints[0]
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: api/routes/pipelex/validate.py
Line: 67-68
Comment:
**Missing `response_model` and return type annotation**
Per the coding standards, all endpoints must declare `response_model` and every function must be fully type-annotated. Both are absent here:
```suggestion
@router.post("/validate", response_model=ValidateResponse)
async def validate_plx(request_data: ValidateRequest) -> JSONResponse:
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: api/routes/pipelex/agent/models.py
Line: 14-15
Comment:
**Missing `response_model` on `/models` endpoint**
Per coding standards, all endpoints must declare `response_model`. The return type `dict[str, Any]` is too loose for a well-typed OpenAPI schema, but at minimum an explicit annotation keeps the codebase consistent:
```suggestion
@router.get("/models", response_model=dict[str, Any])
async def get_models(
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "dev to staging (#42)" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
| ) | ||
|
|
||
| graph_spec, _ = await dry_run_pipeline(mthds_contents=mthds_contents) |
There was a problem hiding this comment.
dry_run_pipeline called outside error-handling block
dry_run_pipeline is called after the try/except ValidateBundleError block, so any exception it raises propagates as an unhandled HTTP 500 rather than returning a clean {"success": false, ...} JSON response — breaking the error contract established for validate_bundle just above.
Suggested change
| graph_spec, _ = await dry_run_pipeline(mthds_contents=mthds_contents) | |
| try: | |
| graph_spec, _ = await dry_run_pipeline(mthds_contents=mthds_contents) | |
| except Exception as exc: | |
| return JSONResponse( | |
| content={ | |
| "success": False, | |
| "mthds_contents": mthds_contents, | |
| "message": str(exc), | |
| } | |
| ) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/routes/pipelex/validate.py
Line: 97
Comment:
**`dry_run_pipeline` called outside error-handling block**
`dry_run_pipeline` is called after the `try/except ValidateBundleError` block, so any exception it raises propagates as an unhandled HTTP 500 rather than returning a clean `{"success": false, ...}` JSON response — breaking the error contract established for `validate_bundle` just above.
```suggestion
try:
graph_spec, _ = await dry_run_pipeline(mthds_contents=mthds_contents)
except Exception as exc:
return JSONResponse(
content={
"success": False,
"mthds_contents": mthds_contents,
"message": str(exc),
}
)
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+154
to
+165
| async def no_auth(request: Request) -> None: | ||
| """No-op auth dependency for AUTH_MODE=none. | ||
|
|
||
| When behind API Gateway, reads identity from forwarded headers | ||
| (X-User-Email, X-User-Sub, X-Auth-Method) set by the Lambda authorizer. | ||
| """ | ||
| email = request.headers.get("x-user-email") | ||
| sub = request.headers.get("x-user-sub") | ||
| auth_method = request.headers.get("x-auth-method") | ||
|
|
||
| if email and sub: | ||
| _set_request_user(request, email=email, sub=sub, auth_method=auth_method or "gateway") |
There was a problem hiding this comment.
no_auth unconditionally trusts the email and identity headers sent by the caller. If the server is reachable directly rather than via an API Gateway, any caller can inject arbitrary values and receive a fully-populated RequestUser. Downstream handlers that use the identity to scope operations (such as the uploader endpoint) will act on the spoofed identity. The docstring documents the API Gateway requirement but provides no runtime enforcement.
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/security.py
Line: 154-165
Comment:
**Forwarded identity headers are spoofable without API Gateway enforcement**
`no_auth` unconditionally trusts the email and identity headers sent by the caller. If the server is reachable directly rather than via an API Gateway, any caller can inject arbitrary values and receive a fully-populated `RequestUser`. Downstream handlers that use the identity to scope operations (such as the uploader endpoint) will act on the spoofed identity. The docstring documents the API Gateway requirement but provides no runtime enforcement.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+84
to
+86
| primary_blueprint = _find_main_blueprint(validate_bundle_result.blueprints) | ||
| if not primary_blueprint: | ||
| primary_blueprint = validate_bundle_result.blueprints[0] |
There was a problem hiding this comment.
IndexError if blueprints is empty
If _find_main_blueprint returns None and validate_bundle_result.blueprints happens to be an empty list, blueprints[0] will raise an IndexError. Adding a guard here makes the error message explicit:
Suggested change
| primary_blueprint = _find_main_blueprint(validate_bundle_result.blueprints) | |
| if not primary_blueprint: | |
| primary_blueprint = validate_bundle_result.blueprints[0] | |
| primary_blueprint = _find_main_blueprint(validate_bundle_result.blueprints) | |
| if not primary_blueprint: | |
| if not validate_bundle_result.blueprints: | |
| return JSONResponse( | |
| content={ | |
| "success": False, | |
| "mthds_contents": mthds_contents, | |
| "message": "No bundle blueprints found after validation", | |
| } | |
| ) | |
| primary_blueprint = validate_bundle_result.blueprints[0] |
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/routes/pipelex/validate.py
Line: 84-86
Comment:
**Potential `IndexError` if `blueprints` is empty**
If `_find_main_blueprint` returns `None` and `validate_bundle_result.blueprints` happens to be an empty list, `blueprints[0]` will raise an `IndexError`. Adding a guard here makes the error message explicit:
```suggestion
primary_blueprint = _find_main_blueprint(validate_bundle_result.blueprints)
if not primary_blueprint:
if not validate_bundle_result.blueprints:
return JSONResponse(
content={
"success": False,
"mthds_contents": mthds_contents,
"message": "No bundle blueprints found after validation",
}
)
primary_blueprint = validate_bundle_result.blueprints[0]
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+67
to
68
| @router.post("/validate") | ||
| async def validate_plx(request_data: ValidateRequest): |
There was a problem hiding this comment.
response_model and return type annotation
Per the coding standards, all endpoints must declare response_model and every function must be fully type-annotated. Both are absent here:
Suggested change
| @router.post("/validate") | |
| async def validate_plx(request_data: ValidateRequest): | |
| @router.post("/validate", response_model=ValidateResponse) | |
| async def validate_plx(request_data: ValidateRequest) -> JSONResponse: |
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/routes/pipelex/validate.py
Line: 67-68
Comment:
**Missing `response_model` and return type annotation**
Per the coding standards, all endpoints must declare `response_model` and every function must be fully type-annotated. Both are absent here:
```suggestion
@router.post("/validate", response_model=ValidateResponse)
async def validate_plx(request_data: ValidateRequest) -> JSONResponse:
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+14
to
+15
| model_type: Annotated[list[str] | None, Query(alias="type", description="Filter by model category: llm, extract, img_gen, search")] = None, | ||
| ) -> dict[str, Any]: |
There was a problem hiding this comment.
response_model on /models endpoint
Per coding standards, all endpoints must declare response_model. The return type dict[str, Any] is too loose for a well-typed OpenAPI schema, but at minimum an explicit annotation keeps the codebase consistent:
Suggested change
| model_type: Annotated[list[str] | None, Query(alias="type", description="Filter by model category: llm, extract, img_gen, search")] = None, | |
| ) -> dict[str, Any]: | |
| @router.get("/models", response_model=dict[str, Any]) | |
| async def get_models( |
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/routes/pipelex/agent/models.py
Line: 14-15
Comment:
**Missing `response_model` on `/models` endpoint**
Per coding standards, all endpoints must declare `response_model`. The return type `dict[str, Any]` is too loose for a well-typed OpenAPI schema, but at minimum an explicit annotation keeps the codebase consistent:
```suggestion
@router.get("/models", response_model=dict[str, Any])
async def get_models(
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Adds agent build endpoints (concepts, pipe specs), a models listing API, and async pipeline runs via Temporal with optional webhook/storage delivery and HMAC. Expands model support (OpenRouter, Linkup), standardizes API inputs to arrays, unifies auth with
AUTH_MODE, adds/api/v1/api_version, and introduces dev/staging deploy workflows.New Features
pipelex-api-ecr-push-production;make deploy-api(prod) andmake deploy-api-staging; Docker image bundles.pipelexdefaults.search)./api/v1/api_version.mthds_contents: list[str]across validate/build/runner/pipeline;bundle_uri→bundle_uris.openrouterbackend andlinkupsearch backend; new search deck and routing profiles; LLM defaults updated (claude-4.6-opus,gpt-4o-mini); expanded catalogs; standardizedthinking_modemetadata across backends.AUTH_MODE(none,api_key,jwt); request user exposed to routes and used in uploads./api/v1; builder (inputs/outputs/runner), validate, model listing, and async run docs updated.Migration
mthds_contents: [content]and handlebundle_urisarrays.AUTH_MODEand provideAPI_KEYorJWT_SECRETas needed.AWS_ACCOUNT_IDand use new role names; usemake deploy-api(prod) ormake deploy-api-staging.OPENROUTER_API_KEY,LINKUP_API_KEY) if using new backends./build/pipeendpoint removed; use agent build endpoints instead.Written for commit 3051754. Summary will update on new commits.