UN-1924 [FIX] Reject unsupported files in API deployment

[Deepak-Kesavan]

What

Reject unsupported files in API deployments at the staging step, by detecting the MIME type from the file's own bytes instead of trusting the caller-supplied Content-Type.

A rejected file is no longer written to the API storage bucket and is no longer dispatched for processing. It is reported back to the caller as its own failed entry naming the offending type. Verified live on the dev env:

{ "execution_status": "COMPLETED",
  "result": [{ "file": "evil.pdf", "status": "Failed",
    "error": "Rejecting file 'evil.pdf' with unsupported MIME type 'text/html'" }] }

Why

SourceConnector.add_input_file_to_api_storage is the single funnel through which API-deployment uploads reach the bucket, and its MIME check read file.content_type — the multipart Content-Type supplied by the caller, which nothing verifies. It also fell back to application/octet-stream when that header was absent, and octet-stream is itself in AllowedFileTypes. Between the two, effectively any file passed the check.

The consequences, both reported on the ticket:

1. Unsupported files reached LLMWhisperer and failed there, rather than being rejected early on the Unstract side.
2. When the declared type was unsupported, the file was skipped from staging but still dispatched under a placeholder temp-hash-… with is_executed=True. The worker then read a path that had never been written, copied 0 bytes, and raised EmptyFileError — so a wrong-file-type upload surfaced as a misleading "empty file" error.

The filesystem/ETL source path already sniffs with libmagic (source.py, and _copy_filesystem_file in the workers). The API path was the asymmetry.

How

Commit 1 — sniff the bytes.

• Added SourceConnector._detect_uploaded_file_mime_type, which reads the leading 8 KiB of the upload, rewinds, and classifies with magic.from_buffer(..., mime=True). libmagic only needs the leading bytes, so this does not pull large uploads into memory.
• Staging validates that sniffed type against AllowedFileTypes before any bytes are written.
• A rejected file is logged via workflow_log.log_error and pushed to ResultCacheUtils.update_api_results as a FileExecutionResult carrying the error. Both the async status endpoint and the synchronous timeout > 0 wait read that same cache, so the entry surfaces either way.
• Removed the placeholder-hash branch and the now-unused uuid import.

Commit 2 — don't strand an execution when everything is rejected.

Dropping rejected files from the dispatch set means that set can now be empty, which reaches a path that was previously unreachable. _unified_api_execution in the API worker short-circuits an empty file set and returns status: "COMPLETED" but never calls update_workflow_execution_status, so the row keeps the status it was dispatched with and the caller polls a PENDING execution forever. (The no-files branch in _run_workflow_api that does write COMPLETED sits after this guard and is never reached.)

This was caught by testing against the dev env, not by the unit tests — worth noting for reviewers.

Fixed on both sides:

• DeploymentHelper.execute_workflow skips the dispatch entirely when staging yields nothing, marks the execution COMPLETED via a new WorkflowExecutionServiceHelper.update_execution_completed, releases the rate-limit slot, cleans up the staging dir, and returns the per-file rejection entries.
• The worker's short-circuit now persists the status, so an empty set arriving from any other caller cannot strand an execution either.

Empty uploads are deliberately let through the MIME check: libmagic reports application/x-empty, which is not in the allow-list, and rejecting them there would relabel an empty-file problem as an unsupported-type one. They continue to be reported as EmptyFileError downstream.

application/octet-stream is intentionally left in AllowedFileTypes. Sniffing already closes the reported hole; removing it would also change ETL/filesystem behaviour and risks rejecting valid-but-unrecognised files. Worth a separate discussion if we want to go further.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why.

The MIME change is scoped to add_input_file_to_api_storage, which has two callers: the API-deployment execution path and the workflow "execute" endpoint used from the UI. Behaviour changes only for files that are actually unsupported:

• Files that were already processing successfully are unaffected. They sniff to their real type, which is in the allow-list. Verified live: a genuine PDF still returns Success with full extraction output. A supported file whose Content-Type header was absent or wrong is now more likely to be accepted, since the bytes decide rather than the header.
• Genuinely unsupported files now fail fast, with a clear message, instead of failing later at extraction. This is the intended change. A caller relying on such a file being silently skipped will now see a Failed entry — but it never produced a usable result before either, it produced a misleading EmptyFileError.
• Multi-file requests are not failed wholesale — verified live that a good file and a rejected file in one request return Success and Failed respectively.
• The empty-dispatch fix is strictly a bug fix. It only changes behaviour for an execution with no files to process, which previously hung in PENDING.
• The worker change touches only the if not converted_files short-circuit; the normal path is untouched.
• Files libmagic cannot identify still pass. Anything that sniffs to application/octet-stream is allow-listed, unchanged. What a given build reports for a plain zip varies — measured as application/octet-stream on one libmagic and application/zip on another — so a zip is allowed on the former and rejected on the latter. Whichever it is, the verdict now comes from the file's own bytes rather than from its declared type.
• Legacy Office uploads were briefly regressed and are fixed. Sniffing only the leading 8 KiB reported application/x-ole-storage for every .doc/.xls/.ppt larger than that window, which is not allow-listed — libmagic resolves OLE2 through a directory sector at the end of the file. Container types are now escalated to a full-file classification. Reproduced on a 710 KB .doc and a 1.2 MB .xls, and pinned by tests.

Database Migrations

• None.

Env Config

• None.

Relevant Docs

• The parent story asks for docs to be updated for unsupported-file behaviour. The user-facing supported-file-type list lives outside this repo, so it is not touched here and is left to the parent ticket.

Related Issues or PRs

• https://zipstack.atlassian.net/browse/UN-1924
• Parent story: https://zipstack.atlassian.net/browse/UN-1923 — sibling sub-tasks UN-1925 (reject in LLMWhisperer) and UN-1926 are already closed; this is the remaining Unstract-side early exit.

Dependencies Versions

• No new dependencies. python-magic==0.4.27 was already declared in backend/pyproject.toml and already imported by this module.

Notes on Testing

Unit tests. backend/workflow_manager/endpoint_v2/tests/test_api_storage_mime_validation.py exercises the real SourceConnector.add_input_file_to_api_storage with its DB/storage collaborators patched. MIME detection is deliberately not mocked — libmagic sniffing is the behaviour under test — and the fixture bytes were chosen against what libmagic actually reports. Covers: a real PDF is staged; an HTML document announced as application/pdf is not staged, not written, not dispatched; the rejection reaches the caller with the offending type and status Failed; a supported file with no declared Content-Type is still staged as application/pdf; a supported file alongside a rejected one still processes.

These were checked to actually discriminate: reverting only the detection line to file.content_type makes 4 of the 5 fail, including evil.pdf being staged to the bucket — the reported bug reproduced as a test.

backend/api_v2/tests/test_deployment_helper.py adds a test that an all-rejected request reaches a terminal status without dispatching, and still returns the rejection entries.

8 unit tests pass locally; CI is green on unit, integration and e2e.

Live verification against an API deployment on the dev env (deepak-unstract-dev), after deploying this branch:

Upload	Result
Genuine PDF	COMPLETED — good.pdf: Success, full extraction output
HTML bytes named .pdf, part declared application/pdf	COMPLETED — evil.pdf: Failed, "Rejecting file 'evil.pdf' with unsupported MIME type 'text/html'"
Both in one request	COMPLETED — good.pdf: Success, evil.pdf: Failed

The spoofed file is the important one: the multipart part explicitly declares application/pdf, so it defeats any header-based check.

Screenshots

Not applicable — no UI surface. API responses are inline above.

Checklist

I have read and understood the Contribution Guidelines.

[greptile-apps]

Greptile Summary

This PR validates API-deployment uploads using MIME types detected from file bytes and reports unsupported files without staging or dispatching them.

• Escalates OLE and ZIP containers to full-file classification so supported Office formats remain identifiable.
• Finalizes all-rejected executions without dispatch and preserves their cached per-file failures.
• Persists terminal status for empty worker inputs and reports total conversion failure as an execution error.
• Corrects API rate-limit slot release to use the organization identifier.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
backend/workflow_manager/endpoint_v2/source.py	Detects upload MIME types from file contents, performs full-file container inspection, rejects unsupported inputs before storage, and records per-file failures.
backend/api_v2/deployment_helper.py	Finalizes all-rejected requests without worker dispatch and releases rate-limit slots using the organization identifier.
backend/workflow_manager/workflow_v2/execution.py	Adds durable completion and aggregate counter updates for executions with no dispatchable work.
workers/api-deployment/tasks.py	Persists terminal outcomes when no files survive conversion or no files were dispatched.
backend/api_v2/tests/test_deployment_helper.py	Covers all-rejected terminalization, cleanup after status-write failure, and normal dispatch when files remain.
backend/workflow_manager/endpoint_v2/tests/test_api_storage_mime_validation.py	Covers content-based MIME validation, rejection reporting, mixed uploads, container inspection, stream preservation, detection failure, and empty files.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[API upload] --> B[Read leading bytes]
  B --> C{Empty file?}
  C -->|Yes| D[Stage as octet-stream]
  C -->|No| E[Detect MIME type]
  E --> F{Container MIME?}
  F -->|Yes| G[Classify full file]
  F -->|No| H{Allowed MIME?}
  G --> H
  H -->|Yes| I[Write to API storage]
  I --> J[Dispatch workflow]
  H -->|No| K[Cache failed file result]
  K --> L{Any staged files?}
  L -->|Yes| J
  L -->|No| M[Persist COMPLETED with failed counts]
  M --> N[Release rate-limit slot and return failures]

Loading

_{Reviews (4): Last reviewed commit: "UN-1924 [FIX] Fail one undetectable uplo..." | Re-trigger Greptile}

[athul-rs]

Standardized review — INITIAL, 16/16 lenses (unstract plugin v0.18.1), head 7af4732 vs base 62a41e9.

Verdict: BLOCK — Critical: 1 · High: 4 · Medium: 8 · Low: 2

The 1 Critical and 4 High findings are posted inline below. The 8 Medium and 2 Low are held out of this comment to keep the thread focused; happy to post them on request. Headline items among them: MIME detection is unguarded so one unreadable stream fails the whole batch (source.py:1203-1210); the early return never calls _set_result_acknowledge, so a later GET /status re-serves the same results with 200 instead of 406; release_slot(api.organization, ...) is a silent no-op (key is built from str(organization.organization_id)), the trap documented at undispatched_sweep.py:245-253; and two comments describe pre-change behaviour, including one this PR's own commit 2 invalidated.

Lens checklist — 1 see #1 · 2 medium · 3 see #1,#2,#3 · 4 clean · 5 see #5 · 6 clean · 7 see #1 · 8 see #2 · 9 clean · 10 see #3,#5 · 11 see #1 · 12 N/A · 13 see #4 · 14 clean · 15 medium · 16 see #1

Lenses 4, 6, 9, 11, 14 were assessed directly rather than by a specialist agent: sniffing replaces a caller-controlled header at a trust boundary and is strictly stronger; staging is synchronous pre-dispatch with no new shared state; an 8 KiB read is cheaper than every existing sniff site; python-magic==0.4.27 is already declared and pinned. Lens 11 is the exception — there is no flag or rollout gate on a change that flips accept/reject on the main upload path, which is what makes finding #1 expensive to unwind.

[athul-rs]

[Critical] [Lens 3 · 1 · 16] — the 8 KiB sniff window rejects legacy Office uploads that work today

Failure mode. libmagic resolves an OLE2 compound file through a directory sector that normally sits near the end of the file. Given only these 8192 bytes it falls back to application/x-ole-storage, which is not in AllowedFileTypes (the list carries application/msword, application/vnd.ms-excel, application/vnd.ms-powerpoint, application/CDFV2 — enums.py:19,21,24,28). Every .doc/.xls/.ppt larger than the sample is therefore rejected as unsupported, on both callers of add_input_file_to_api_storage: the API-deployment path (deployment_helper.py:280) and the UI workflow execute endpoint (workflow_v2/views.py:263). Before this PR the caller-declared type was accepted and the file processed.

Evidence. Reproduced twice independently, on a real 248 KB .ppt and on LibreOffice-produced .doc/.xls, with libmagic 5.45 and with 5.46 inside the shipped backend image:

sample4.ppt   8KiB -> application/x-ole-storage   64KiB -> application/x-ole-storage   full -> application/vnd.ms-powerpoint
doc500.doc    8KiB -> application/x-ole-storage                                        full -> application/msword
big.xls       8KiB -> application/x-ole-storage                                        full -> application/vnd.ms-excel

Widening to 64 KiB does not fix it. The same root cause degrades a .docx whose [Content_Types].xml compresses past the window to application/zip, also not allow-listed.

Every other sniff site in this codebase reads 4 MiB (source.py:75, workers/shared/workflow/execution/service.py:1220), which is why these files pass the existing downstream check and fail only the new one — 8 KiB is 512x narrower and introduced here.

This also makes the comment on line 76 false, and it is the stated justification for the constant. The PR description's "files that were already processing successfully are unaffected — they sniff to their real type, which is in the allow-list" does not hold for this class.

Suggested fix. Sniff the full staged object (magic.from_file), or treat application/x-ole-storage and application/zip as inconclusive rather than unsupported. Either way the regression test needs a fixture larger than the window — the current ones are 45 bytes (tests/test_api_storage_mime_validation.py:27-28), which is why neither CI nor the dev-env run surfaced this.

Confidence: High.

[athul-rs]

[High] [Lens 3 · 8] — this branch can raise before its own cleanup runs

Failure mode. update_execution_completed catches only WorkflowExecution.DoesNotExist (execution.py:393-401). Any other DB failure — OperationalError, a statement or lock timeout on the select_for_update inside update_execution (models/execution.py:418-423), a deadlock, a dropped connection — propagates out of execute_workflow, so line 315 and lines 316-318 never run. The org's rate-limit slot stays held for the full 6h TTL and throttles every other API-deployment call for that org, the staging dir is never deleted, the row stays PENDING, and the caller gets a 500 with no execution id to poll.

Evidence. The sibling block immediately above (lines 289-308) deliberately isolates its DB write in an inner try/except with logger.exception so that cleanup always runs, and has a regression test pinning exactly that: test_staging_failure_cleanup_survives_db_marking_error (tests/test_deployment_helper.py:75-91). The new path copies the shape but not the guard, and has no equivalent test.

Suggested fix. Wrap the update_execution_completed call in its own try/except Exception: logger.exception(...) so release_slot and delete_api_storage_dir always execute, and add the mirror-image test.

Confidence: High.

[athul-rs]

[High] [Lens 3 · 10] — the response asserts COMPLETED whether or not the status write landed

Failure mode. There are three ways the call on line 314 returns normally without the row reaching COMPLETED:

1. row missing — execution.py:399-401 logs and returns None;
2. row vanished under the lock — models/execution.py:424-425, if locked is None: return, silent;
3. row already terminal with a different value — models/execution.py:520-535 refuses, logs a warning, returns ([], False).

The return value is discarded and execution_status on line 323 is a hardcoded literal rather than the row's actual status. The API then answers COMPLETED while a follow-up GET /status/<execution_id> reads the DB and returns PENDING — the stranded-execution bug this PR exists to fix, now concealed behind a success response instead of being visible.

update_execution_completed was given a WorkflowExecution | None return type to carry exactly this signal, and no caller reads it.

Suggested fix. Bind the result: if it is None, or its status is not COMPLETED, log at error level and return the row's real status (or ERROR) rather than claiming COMPLETED.

Confidence: High.

[athul-rs]

[High] [Lens 13] — the guard's predicate is unpinned; the original bug can be reintroduced with the suite green

Failure mode. This test passes file_objs=[] with SourceConnector fully mocked, so it exercises the zero-files-uploaded path, not the zero-files-staged path the guard exists for. Nothing in the suite asserts that a non-empty upload whose staging result is empty takes the short-circuit, and nothing asserts the short-circuit does not fire when staging returns files.

Evidence (mutants run against the branch, then reverted):

• if not hash_values_of_files: -> if not file_objs: at deployment_helper.py:313 — 3/3 pass. That mutant is the production bug verbatim: one HTML file uploaded, staging rejects it and returns {}, file_objs is non-empty, control falls through to execute_workflow_async, execution stranded in PENDING.
• if not hash_values_of_files: -> if True: — the whole backend/api_v2/tests/ suite is identical to baseline (48 passed).
• Control: deleting the update_execution_completed call does fail this test, so it pins the branch body, not the branch condition.

Also worth noting: assert response["result"][0]["status"] == "Failed" on line 148 reads back the fixture's own literal from line 115, so it proves the branch forwards the cache verbatim, not what source.py writes.

Suggested fix. Pass a non-empty file_objs (a bare MagicMock() suffices — with SourceConnector mocked, the only read is len(file_objs) at deployment_helper.py:243) so the two cases become distinguishable, and add a sibling test with add_input_file_to_api_storage.return_value = {"good.pdf": MagicMock()} asserting execute_workflow_async is called and update_execution_completed is not. Parametrising timeout over {-1, 10} closes the untested synchronous path at negligible cost.

Confidence: High (mutants executed).

[athul-rs]

[High] [Lens 5 · 10] — a rejected file leaves no durable record, and an all-rejected run is stored as a clean success

Failure mode. Before this change a rejected file produced a FileHash, was dispatched, and the worker's own libmagic check (workers/shared/workflow/execution/service.py:1223-1229) created a real WorkflowFileExecution row. After this change it produces no FileHash, no WorkflowFileExecution, no file-history row. The rejection exists only as an entry in the Redis list api_results:{workflow_id}:{execution_id}, which is deleted the first time anyone polls /status (workflow_helper.py:451-453), expires after EXECUTION_RESULT_TTL_SECONDS (3h default), and is gone on any eviction or restart. After any of those, nothing in Postgres can answer "why was my file not processed?".

Compounding it on the all-rejected path: deployment_helper.py:313-318 writes only status=COMPLETED. The row keeps total_files = len(file_objs) from line 243 while failed_files and successful_files stay NULL (models/execution.py:191-208, nullable, no default). is_failure_run is is_failure(status) or (failed_files or 0) > 0 (unstract/core/.../data_models.py:663), so COMPLETED + NULL reads as a success — the response body says every file Failed while the execution row says N files, zero failures. This is the hazard already written up at internal_views.py:546-550 ("a terminal status with failed_files=None ... silently bypasses notify_on_failures subscribers"). Run history is affected too: get_last_run_statuses derives PARTIAL_SUCCESS from these counters (models/execution.py:622-636).

Separately, the early return never reaches PipelineUtils.update_pipeline_status, the only dispatcher of API-deployment notifications (pipeline_utils.py:58 -> APIDeploymentUtils.send_notification), so an all-rejected request now sends no webhook at all where the dispatched-and-failed run previously alerted.

Note also that the worker-side check already raises UnsupportedMimeTypeError naming the file and the MIME type, which softens the PR description's premise that an unsupported file today "fails at extraction with an error that does not name the real cause".

Suggested fix. Write the aggregates alongside the status (failed_files=len(file_objs), successful_files=0), and keep a persisted per-file record for a rejected file — a WorkflowFileExecution row in terminal ERROR carrying the real MIME type — so the rejection is auditable after the cache entry is gone. If cache-only is deliberate, it is worth stating in the PR description as a support/audit trade-off.

Confidence: High.

[chandrasekharan-zipstack]

PR Review — Standardized (LITE)

Verdict — REQUEST CHANGES

Mode: LITE — single-pass, 16/16 lenses, one reader, no subagents. Eligible: 6 files / 309 lines, no disqualifiers.

Summary — Critical: 0 · High: 2 · Medium: 3 · Low: 1 · Lenses run: 16/16

The core change is right. Sniffing bytes instead of trusting the multipart Content-Type is the correct fix, it matches what the filesystem/ETL path already does, and the test that stages evil.pdf is a genuine reproduction of the reported bug. The findings are all in commit 2 — the empty-dispatch handling — plus one behaviour claim in the PR body that does not hold.

Findings are posted as inline comments (#1–#6).

Lens checklist

#	Lens	Result
1	Spec & intent	See finding #4
2	Architectural fit & precedent	Clean — mirrors the libmagic sniffing already in the filesystem source path; reuses ResultCacheUtils / FileExecutionResult rather than inventing a reporting channel
3	Correctness & edge cases	See findings #1, #3
4	Security	Clean — this closes a spoofed-Content-Type hole; no authn/authz/tenancy surface touched
5	Data integrity & migrations	N/A — no schema, migration, or backfill; the status write routes through the existing guarded model method
6	Concurrency	Clean — release_slot is zrem (rate_limiter.py:356-357), so the explicit release plus update_execution's own terminal release is not a double-decrement
7	API & contract compatibility	See finding #4 — response shape unchanged, per-file status semantics change
8	Reliability & resilience	See finding #1
9	Performance & cost	Clean — 8 KiB read + rewind per upload, before any write. Verified DOCX/XLSX/PPTX, CSV, JSON and PDF all classify correctly from the first 8 KiB
10	Observability	Clean — rejection logged via workflow_log.log_error and surfaced to the caller; no PII added
11	Operational safety	See finding #3; no flag or rollout surface in this diff
12	LLM/agent-specific	N/A — no model call, prompt, tool config, or eval touched
13	Testing	See finding #5
14	Dependencies & build	N/A — no dependency change; python-magic==0.4.27 already declared and already imported at source.py:13
15	Code quality	See finding #6
16	Doc & comment accuracy	See finding #2

Open questions

1. Finding #1 — was the conversion-failure case (non-empty input, empty converted_files) considered, or is the guard only meant for the genuinely-empty set the backend now produces?
2. Finding #4 — do you know of tenants pushing zips or RTF through API deployments today? They pass on main when the header is absent or octet-stream, and stop passing after this.
3. The all-rejected path never calls delete_api_results / _set_result_acknowledge, so the rejection entries live until EXECUTION_RESULT_TTL_SECONDS. Intentional (a later status poll still shows them) or an oversight?

Assumptions made

• The libmagic results in finding #4 come from the venv at backend/.venv. If CI or the runtime image ships a different libmagic, that table could shift — the OOXML-from-8KiB result in particular is version-sensitive, though it held here.
• I took CI-green and the dev-env verification in the PR body at face value; I did not re-run the suite.
• Finding #2 assumes API deployments still run on the Celery transport by default (queue_message_id IS NULL). If the PG queue transport is now universal, that one drops to Low.

[chandrasekharan-zipstack]

[High] [Lens 3, 8] — Short-circuit now reports COMPLETED for a conversion failure, not just an empty input

FileProcessingUtils.convert_file_hash_data catches per-file exceptions, logs, and continues (workers/shared/processing/files/utils.py:65-69), returning only what converted. So if not converted_files is reachable with a non-empty hash_values_of_files when every file failed conversion.

The new unconditional write turns that into a successful, zero-file execution: the row goes COMPLETED with total_files=0, the caller gets status: "COMPLETED" with no results and no error, and the only trace is a logger.warning. Before this diff that case stranded in PENDING — also wrong, but loudly wrong. A silent success is the worse of the two.

# utils.py:65-69 — errors are collected and swallowed
except Exception as e:
    conversion_errors.append(error_msg)
    continue
...
return converted_files            # can be {} for non-empty input

The branch's own log line already says the quiet part: "No valid files to process after conversion" — after conversion, not nothing was sent.

Suggested fix — distinguish the two:

if not converted_files:
    if hash_values_of_files:
        api_client.update_workflow_execution_status(
            execution_id=execution_id,
            status=ExecutionStatus.ERROR.value,
            error_message="No files could be converted for processing",
        )
        return {"execution_id": execution_id, "status": "ERROR", ...}
    api_client.update_workflow_execution_status(..., status=ExecutionStatus.COMPLETED.value, total_files=0)

Confidence: High that the branch is reachable with non-empty input. Medium on production frequency — FileHashData.from_dict over backend-produced dicts rarely throws.

[chandrasekharan-zipstack]

[High] [Lens 16] — This comment asserts a guard that does not hold on the Celery transport

"the model method owns the terminal-one-way guard, so this cannot revert an already-final row" is only true on the PG path.

update_execution routes on queue_message_id: when it is NULL (the Celery/legacy transport, still the default), _apply_legacy_update runs — and it sets self.status = status.value unconditionally, with no guard at all (models/execution.py:455-471). Only _apply_guarded_status (models/execution.py:506-517) guards.

Both sibling comments in this same file carry the qualifier this one drops:

• execution.py:176-178 — "…terminal-one-way guard (atomic select_for_update, PG-scoped, field-scoped writes)"
• execution.py:382-384 (update_execution_err) — "…so a late error handler can't revert a PG execution the callback already finalized"

A maintainer trusting this comment and reusing update_execution_completed in a late completion callback would overwrite an already-ERROR execution with COMPLETED.

Suggested fix — comment-only; restore the scoping, e.g. "…so a PG execution the callback already finalized cannot be reverted."

No live bug at the current call site — the row is freshly created and PENDING — which is why this is High and not Critical.

[chandrasekharan-zipstack]

[Medium] [Lens 3, 11] — Cleanup is skipped if this status write raises

update_execution_completed catches only WorkflowExecution.DoesNotExist; the update_execution it calls does select_for_update plus a save() and can raise OperationalError/DatabaseError. That propagates out of execute_workflow, so release_slot and delete_api_storage_dir on the next two lines never run: the org's rate-limit slot stays occupied and the staging dir is orphaned.

The sibling staging-failure path 25 lines above guards against precisely this, and there is a regression test pinning it:

# deployment_helper.py:285-291
try:
    WorkflowExecutionServiceHelper.update_execution_err(...)
except Exception:
    logger.exception(f"Failed to mark execution {execution_id} as ERROR")
# then release_slot + delete_api_storage_dir

backend/api_v2/tests/test_deployment_helper.py:75 — test_staging_failure_cleanup_survives_db_marking_error.

Suggested fix — same try/except Exception: logger.exception(...) wrapper around the update_execution_completed call.

Medium rather than High because the leak self-heals: _cleanup_expired_entries sweeps the zset by score (backend/api_v2/rate_limiter.py:44-47).

[chandrasekharan-zipstack]

[Medium] [Lens 1, 7] — The PR body's compatibility claim about zips is wrong

The description states: "Mislabelled binaries that libmagic reports as octet-stream (e.g. plain zips) still pass, as before". libmagic does not report zips as octet-stream. Measured against the pinned python-magic:

bytes	sniffed	in AllowedFileTypes?
plain zip	application/zip	✗
RTF	text/rtf	✗
HEIC	image/heic	✗
BMP header	application/octet-stream	✓

Any upload of those that previously reached the bucket by declaring application/octet-stream or by omitting Content-Type is now a hard Failed (AllowedFileTypes, backend/workflow_manager/endpoint_v2/enums.py:9-35).

That may well be the intent — the check itself is correct and this is not a code defect. But "Can this PR break any existing features" currently asserts the opposite, so whoever approves this is approving an understated blast radius.

Suggested fix — correct the claim in the PR body. If any tenant is known to push zips/RTF through API deployments, that is a rollout question worth answering before merge rather than after.

Confidence: High on the libmagic behaviour (measured, not recalled). Medium on real-world impact — depends on tenant traffic I cannot see.

[chandrasekharan-zipstack]

[Medium] [Lens 13] — The two deliberate new branches are the two without tests

This empty-upload branch exists specifically to preserve the downstream EmptyFileError path — it is the one branch of _detect_uploaded_file_mime_type reachable only with zero bytes, and nothing pins it. Delete it and the suite stays green while empty uploads start being relabelled as unsupported-type failures, which is the exact outcome the comment says to avoid.

Same for workers/api-deployment/tasks.py:225-231: nothing asserts the worker now persists the status, which is the whole point of that hunk.

Suggested fix — two assertions:

• _stage([_upload("empty.pdf", b"", "application/pdf")]) returns the file staged with mime_type == "application/octet-stream".
• A worker test asserting update_workflow_execution_status is called with COMPLETED on the empty short-circuit.

Noted in the PR's favour: the existing 5 tests were verified to discriminate (reverting the detection line fails 4), which is more than most PRs do.

[chandrasekharan-zipstack]

[Low] [Lens 15] — The two fixes for the same scenario disagree on total_files

The worker sets total_files=0 (workers/api-deployment/tasks.py:230); this branch leaves it at the creation-time len(file_objs) (deployment_helper.py:241). An all-rejected run therefore lands COMPLETED with total_files=1 and zero file executions.

Cosmetic in the API response (which reads the result cache), but the executions list shows a completed run whose counts do not add up.

Suggested fix — have update_execution_completed zero the count, or accept a total_files argument, so both paths agree.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Unstract test results

Per-group results

Status	Group	Tier	Passed	Failed	Errors	Skipped	Duration (s)
✅	e2e-api-deployment	e2e	3	0	0	0	21.4
✅	e2e-coowners	e2e	1	0	0	0	1.6
✅	e2e-etl	e2e	1	0	0	0	8.5
✅	e2e-login	e2e	2	0	0	0	1.5
✅	e2e-prompt-studio	e2e	1	0	0	0	4.9
✅	e2e-smoke	e2e	2	0	0	0	1.5
✅	e2e-workflow	e2e	1	0	0	0	16.6
❌	frontend	unit	0	1	0	0	0.0
✅	integration-backend	integration	310	0	0	26	48.0
✅	integration-connectors	integration	1	0	0	7	8.0
✅	integration-workers	integration	157	0	0	1	51.1
❌	ui	e2e	0	1	0	0	0.0
✅	unit-backend	unit	1171	0	0	1	44.9
✅	unit-connectors	unit	63	0	0	0	10.3
✅	unit-core	unit	33	0	0	0	1.4
✅	unit-platform-service	unit	15	0	0	0	2.8
✅	unit-rig	unit	120	0	0	0	4.7
✅	unit-runner	unit	5	0	0	0	3.0
✅	unit-sdk1	unit	563	0	0	0	29.9
✅	unit-workers	unit	1397	0	0	1	132.1
	TOTAL		3846	2	0	36	392.4

Critical paths

⚠️ Critical paths not yet covered

• workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)

✅ Covered critical paths

• auth-login — covered by e2e-login
• adapter-register-llm — covered by integration-backend
• workflow-author — covered by integration-backend
• co-owner-manage — covered by integration-backend, e2e-coowners
• workflow-create-execute — covered by e2e-workflow
• api-deployment-provision — covered by integration-backend
• api-deployment-auth — covered by integration-backend
• api-deployment-run — covered by e2e-api-deployment
• mcp-server-auth — covered by integration-backend
• mcp-platform-auth — covered by integration-backend
• prompt-studio-author — covered by integration-backend
• prompt-studio-fetch-response — covered by e2e-prompt-studio
• connector-register-test — covered by integration-backend
• pipeline-etl-execute — covered by e2e-etl
• usage-aggregate-read — covered by integration-backend
• usage-token-tracking — covered by e2e-api-deployment
• callback-result-delivery — covered by e2e-api-deployment