Skip to content

[finding] packageService.delete's driver fault is answered as 400 PACKAGE_DELETE_FAILED — the same mislabelling #8131 fixed for publish, on the sibling route #8275

Description

@os-zhuang

Found while implementing #8131 (the publish half of the package door's returned-failure classification). Recorded rather than fixed: #8131's card names POST /packages/publish and the DELETEpartial-failure array, and this is a third returning path it does not name — a distinct route with its own acceptance test. Filed unassigned; nobody is on it.

The gap

packageService.delete swallows a driver error and reports failure by returning, exactly as publish did before #8131 (packages/services/service-package/src/index.ts):

}catch(error){logger.error('Failed to delete package',errorasError);return{success: false};}

and the handler answers 400 (packages/rest/src/package-routes.ts, the version-scoped DELETE branch):

sendError(res,400,'PACKAGE_DELETE_FAILED',`Failed to delete ${packageId}${version ? `@${version}` : ''}.`,);

The statement that failed is DELETE FROM sys_packages WHERE id = ? [AND version = ?]. A driver failure there — a missing table, a lock timeout, a foreign-key restriction — is a server fault answered as a client error: it invites the caller to fix a request that was never the problem, and it hides a real fault from every dashboard that buckets by status. This is the same class #8016 fixed for the throw path and #8131 fixed for publish, in mirror image.

What is NOT wrong here

There is no disclosure. Unlike publish, this path never interpolated the driver text — the producer returns a bare { success: false } and the handler writes its own sentence. So this is a status-classification defect only, and it is not a security finding.

That is also why it was left out of #8131 rather than swept in: the two halves have different acceptance tests, and #8136 explicitly asks that status-classification work not be folded across cards without a triage ruling.

Why it is small but worth doing

#8131 converted publish and left delete in the older shape, so service-package is now a partially converted producer — the state package-envelope.conformance.test.ts's own header calls "arguably worse than untouched", because the same service answers two different classifications for the same kind of fault.

Suggested shape (matching what #8131 landed)

The remedy is the one already in the file next door, and it is roughly three lines:

⛔ Note the discriminant must be the status channel, not .code: every SQL driver populates a string code (ERR_SQLITE_ERROR, 42P01, ER_NO_SUCH_TABLE), and #8131 measured that reading it re-throws genuine driver faults into a 500 whose message the leak heuristic does not withhold. packages/services/service-package/src/publish-driver-fault.test.ts pins that per dialect.

Acceptance should assert bothcode and status per ADR-0112, and keep a pin that a declared refusal on this route still answers 4xx.

Related

#8131 (the publish half, landed) · #8016 · #8086 · #8136

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions