npm remote 304 bumps advance the freshness clock on a soft-deleted parent

Both npm-remote 304-revalidation bumps advance upstream_checked_at on a cache row whose parent has been soft-deleted. The bump reports nil, so a document or tarball belonging to a package the operator suppressed reads as freshly revalidated.

Affected methods

  • datastore.NpmRemoteFileStore.BumpUpstreamCheckedAt — gates npm_remote_files.soft_deleted_at IS NULL, which covers the file row's own tombstone and nothing above it. A soft-deleted parent npm_remote_versions row does not stop the bump.
  • datastore.NpmRemoteMetadataFileStore.BumpUpstreamCheckedAt — gates nothing beyond the row's own identity. npm_remote_metadata_files has no soft_deleted_at column, and the npm_remote_packages foreign key declares no ON DELETE action. A soft delete is an UPDATE, so nothing would cascade even if it did.

The asymmetry between the two WHERE clauses is about each row's own tombstone. Neither addresses the parent.

Why the read path does not cover it

NpmRemotePackageByName filters npm_remote_packages.soft_deleted_at IS NULL and is the only production source of the package id, so a suppressed package never yields an id to read metadata with. The bump does not go through it — the row id arrives from the Lookup the handler already performed, and the exposure window is Lookup → upstream round trip → bump.

NpmRemoteMetadataFileStore's own read-path doc comment already names this hazard:

Sourcing the ID any other way would bypass that gate and serve — or revalidate and re-serve — metadata for a package the operator suppressed.

Reproduction

Verified against a live PostgreSQL instance:

  1. Seed a namespace, remote repository, package, and metadata file (or version and tarball file).
  2. Soft-delete the parent npm_remote_packages row (or npm_remote_versions for the tarball side).
  3. Call BumpUpstreamCheckedAt with the child row's id.

Observed: returns nil, and upstream_checked_at advances. Expected: ErrNotFound, and the clock does not move.

Why this is latent today

No production code soft-deletes any npm-remote row. The only non-test writer of a soft delete is SoftDeleteNpmVersion, on the hosted npm_versions table. The gap becomes reachable as soon as the first npm-remote soft-delete writer lands — which is the reason for tracking it rather than leaving it in a doc comment, since whoever writes that code has no reason to be reading BumpUpstreamCheckedAt.

Proposed fix

Add a correlated EXISTS over the parent, filtered on soft_deleted_at IS NULL, to both WHERE clauses. That feeds the existing affected == 0 branch, so it needs no new error vocabulary and no change to the remote.CacheStore seam contract — a suppressed parent surfaces the same ErrNotFound a vanished row does, and the adapter maps it to remote.ErrCacheEntryNotFound as it already would.

Tests: neither bump suite currently covers a soft-deleted parent. Add one subtest per store asserting ErrNotFound plus a readback proving the freshness clock did not advance.

Follow-up in the code

NpmRemoteMetadataFileStore.BumpUpstreamCheckedAt's doc comment carries the full description of this gap, and NpmRemoteFileStore.BumpUpstreamCheckedAt points at it. Both should be retargeted to reference this issue instead of describing the fix inline.

Found during review of !1188 (merged)