feat(csam): record which archive path actually ran (#4873)

* feat(csam): record which archive path actually ran

Since the streaming-upload flag went live, the evidence archiver has had two
media-archive paths and nothing recorded which one executed. Archives completed
cleanly and "did the streaming path run?" was unanswerable: the job emitted no
path-identifying log line, and every existing csam metric is monitor-level
(pending / scanned / oldest-age / last-success), so none of them describes HOW an
archive completed. The flag being on is not evidence the flagged branch ran.

Adds the two halves of that answer:

  - one logToAxiom line at the DECISION point, not at completion. An archive that
    dies partway never reaches either terminal state, so a completion-only record
    would be silent for exactly the failures worth investigating. This line is
    the one that survives a killed container.
  - civitai_csam_archive_total{path,type,outcome}, incremented at both terminal
    states.

path is what the CODE did, not what the flag said. The flag governs archiveImages
and archiveGeneratedImages only, so a TrainingData report (which downloads a
prebuilt file) and an ExternalLink report (which archives no media) resolve to
"none" however the flag is set. Labelling those "stream" because the flag was on
would make the series a record of the flag rather than of the code, which is the
confusion this counter exists to end. csamArchivePathFor is the single place that
mapping is expressed, so the log line and the counter cannot disagree.

Cardinality is 12 series, and every one is REACHABLE. The naive product is 3x4x2
= 24, but half of those pairs the code cannot produce. Seeding an impossible pair
would put a permanent zero on screen that no code path can ever move, which reads
as "this never happens" when it means "this cannot happen". REACHABLE_SERIES is
derived from csamArchivePathFor rather than written out, so the two cannot drift.

All 12 are seeded at 0, and the ensureRegister call in /api/metrics is the other
required half. CSAM archives are rare, so without seeding a pool that has not
archived recently exposes nothing and a PromQL read returns no data --
indistinguishable from an unwired instrument, which is the precise ambiguity
being fixed. Both halves are required; neither works alone.

ALERTING: use max_over_time(), never rate()/increase(). A pod that archives once
sets its child to 1 and never moves it again, so a rate() over that child is
structurally 0 and an alert keyed on it silently never fires.

The emitter is fail-soft and drops unknown label values: this instruments a
legally-mandated evidence path, so a metrics error must never turn an archive
that succeeded into one that failed.

Verification: 9 unit tests against the real prom-client default registry (not a
mock, so a metric-name or label typo cannot pass). Mutation-tested -- four
mutants, all killed: widening the flag-governed set, deleting the validity guard,
deleting the seeding call, and making the emitter rethrow. The guard and
fail-soft mutants each killed exactly the one test that names them. Typecheck
reports no error in any changed file; the changes to existing files are purely
additive (55 insertions, 0 deletions).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0181xaj7W1xEgKbvRNTP65eD

* fix(csam): narrow the path claim, pin the enum, guard the seam

Round 1 audit findings. Five 🟡, no 🔴.

1. The module header asserted in caps that `path` is what HAPPENED, not what the
flag said. True on the success arm -- archiveAndUpload branches on the flag with
no internal fallback -- but FALSE on the error arm: archivePath is resolved
before any archiving begins, so a failure upstream of the media archive records
the selected path though that branch never ran. The claim was wider than the
code. Narrowed rather than re-justified: the header, the help text and the
catch-site comment now say per-arm what the label means, and state plainly that
{path=stream,outcome=error} does not establish that streaming failed. The
genuine improvement over reading the flag is the `none` mapping, and that is now
what the header claims.

2. The seam was unguarded. Deleting BOTH recordCsamArchive calls left the whole
csam suite green: every test that loaded the service asserted on archives, every
test that loaded the counter called it itself, and nothing pinned that the
service invokes the counter at all. Adds four cases to
csam-archive-stream-upload.test.ts, which already builds the end-to-end fixture.
They assert a RELATIONSHIP, not a component. Mutation-verified: deleting both
calls kills all four; swapping success/error at the two sites kills all four;
passing the flag through instead of the resolved path kills exactly the
path=none case -- the one that names that property.

3. `report.type as CsamArchiveType` was redundant (CsamReportType is the
identical union, so it type-checks uncast) and was the only thing that would
have masked Prisma enum drift. Removed all three casts, and added a
compile-time pin against CsamReportType that fails the build in BOTH
directions. Watched it fail: adding a member to our union errors TS2344 at one
branch, removing one errors at the other, and it is silent when they agree.
Without this a new report type would resolve to path=none, log "via the none
path", and be DROPPED by the counter with no error, no log and no failing test.

4. The help text said "attempts that reached a terminal state"; there are three
terminal states and it counted two. A report with no reported user returns
before the flag is read, and a scratch-volume mkdir failure throws before the
try block. The text now names both exclusions. The log line also moved ABOVE the
createDir loop, so the mkdir failure -- the disk-pressure class this whole change
exists to observe -- is no longer completely invisible. The counter still cannot
see it, which the help text now says rather than leaving it to be discovered.

5. Three comments carried a CSAM caseload figure into public source. Removed;
each site makes the same argument saying "rare". The runtime log line is
unchanged -- an internal sink, consistent with the four existing csam-report log
calls -- and the observability internals in prose were trimmed with it.

Not changed, deliberately: the unguarded ensureRegister at module scope in
/api/metrics (its two neighbours have identical exposure -- pre-existing
convention, not this PR's to change alone), and the ordering of the DB write
before the error counter in the training-data branch.

Verification: 41/41 across the three csam suites. Both mutation batteries re-run
after the edits, all mutants still killed for their own reason. eslint clean on
every changed file, prettier clean, typecheck unchanged at 27 pre-existing
errors, none in any file this PR touches.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0181xaj7W1xEgKbvRNTP65eD

* fix(csam): correct three claims the last fix round wrote

Round 2 delta audit: 1 🟡, 2 🟢, no 🔴. All three are prose this PR's own
previous round authored while fixing round 1 — none is a code defect.

F1 (🟡, and it shipped in the scrape output). The help string said a
scratch-volume mkdir failure is visible "only in the log line", and the service
comment said emitting first "stops that failure being completely invisible".
Both false. process-csam.ts's per-report catch already logs subType=archive-data
carrying errorMessage(e), which for the mkdirSync throw is the real errno. So it
was never invisible. What the move actually buys is ATTRIBUTION: that existing
record carries no reportId, reportType or archivePath, so before the move a
mkdir failure could not be tied to a report or to the path it was routed to.
Corrected to say that, rather than reaching for a different justification -- and
the help text now points at subType=archive-data, which is the record that names
the errno, instead of at the info line that names nothing.

F2 (🟢). Two sentences in the same commit answered "does scratch-volume trouble
reach this counter?" oppositely -- the header listed a scratch-volume WRITE among
failures that ARE counted, the help text said a scratch-volume MKDIR is NOT.
Both true, neither distinguishing. Both now say which one they mean.

F3 (🟢). The seam block cited "the entire csam suite green (42/42)" as a
measurement. It does not reproduce: the auditor got 50/50, and the figure came
from a report over a different file set than the sentence implies. Replaced with
what is actually measured here, scoped to the three named files -- all 37
pre-existing tests stay green and only the four new cases go red -- plus a note
that a wider file set gives a different total and the total is not the claim.

Also records, at the relocation site, why the obvious alternative is wrong:
moving the createDir loop inside the try would make the catch's cleanup throw,
because removeDir is fs.rmSync without `force` and ENOENTs on a directory that
was never created. Written down so a later round does not "fix" it.

Verification: 54/54 across all four csam suites. The seam mutation battery
re-run -- deleting both emitter calls and swapping success/error each kill all
four seam cases, flag-passthrough kills exactly the path=none case -- which is
also the re-measurement behind F3's corrected sentence. The enum pin re-proven
under the repo's own gate (node scripts/typecheck.mjs), TS2344 in both drift
directions and silent when the unions agree, at an unchanged 27-error baseline
with none in a file this PR touches; that was the one uncertainty round 2 left
open, having compiled a single file under a scratch tsconfig instead. eslint and
prettier clean on every changed file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0181xaj7W1xEgKbvRNTP65eD

* docs(csam): say NON-SEAM, not PRE-EXISTING, in the seam measurement

9 of those 37 live in the metrics test file this PR itself added, so "pre-existing"
is true only relative to the seam commit, not to origin/main. The numbers and the
argument are unchanged -- the seam case is in fact stronger under the origin/main
reading. Comment-only; taken from the round-3 auditor's own suggested wording after
that round returned clean, and not re-audited for it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0181xaj7W1xEgKbvRNTP65eD

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Zachary Lowden
2026-09-16 12:52:02 -05:00
committed by GitHub
parent 9478ba23de
commit 027cec499a
5 changed files with 605 additions and 4 deletions
+11
View File
@@ -37,10 +37,21 @@ import { ensureRegisterGenerationModelSubstitutionMetrics } from '~/server/metri
// never rescued anyone here" would then be the same observation, which is precisely
// the ambiguity that counter was added to remove.
import { ensureRegisterImageUploadRelayMetrics } from '~/server/prom/image-upload-relay.metrics';
// Same reason as the two neighbours above, and the case where it matters MOST: seeds all
// 12 reachable (path, type, outcome) series of civitai_csam_archive_total at 0. CSAM
// archives are RARE — so on
// almost every pod, for almost all of its life, the honest reading of this counter is a
// row of zeros. prom-client materialises a child only on its first inc(), so without this
// `…{path="stream"}` returns `no data`, and "streaming has never run here" would be
// indistinguishable from "the instrument is not wired" — which is the precise ambiguity
// this counter was added to end, so leaving it unseeded would reproduce the defect one
// level down.
import { ensureRegisterCsamArchiveMetrics } from '~/server/metrics/csam-archive.metrics';
import { WebhookEndpoint } from '~/server/utils/endpoint-helpers';
ensureRegisterGenerationModelSubstitutionMetrics();
ensureRegisterImageUploadRelayMetrics();
ensureRegisterCsamArchiveMetrics();
const labels: Record<string, string> = {};
if (process.env.PODNAME) {
@@ -0,0 +1,198 @@
import client from 'prom-client';
import { beforeEach, describe, expect, it } from 'vitest';
import {
CSAM_ARCHIVE_OUTCOMES,
CSAM_ARCHIVE_PATHS,
CSAM_ARCHIVE_TYPES,
REACHABLE_SERIES,
csamArchivePathFor,
ensureRegisterCsamArchiveMetrics,
recordCsamArchive,
} from '../csam-archive.metrics';
/**
* The REAL prom-client side of the CSAM archive-path signal.
*
* Three failure classes are pinned here that a service-level test structurally
* cannot see:
*
* 1. A metric-name or label-name typo. That sails through every test that mocks
* this module and yields a dashboard/alert that silently never fires — the
* exact class this signal exists to prevent. So this file drives the real
* default registry.
* 2. 🔴 A throw escaping into the CSAM archive path. This emits on a SUCCESSFUL
* archive of legally-mandated evidence, so an unguarded prom-client error
* would turn a completed archive into a failed one — observability causing
* the outage it watches for.
* 3. 🔴 The path mapping drifting from the flag semantics. `path` must describe
* what the CODE did, not what the flag said: the flag governs only
* `archiveImages`/`archiveGeneratedImages`, so the other two report types
* must read `none` no matter how it is set.
*/
const NAME = 'civitai_csam_archive_total';
function freshRegistry() {
const reg = new client.Registry();
ensureRegisterCsamArchiveMetrics(reg);
return reg;
}
async function seriesOf(reg: client.Registry) {
const metric = (await reg.getMetricsAsJSON()).find((m) => m.name === NAME);
if (!metric) throw new Error(`${NAME} is not registered`);
// `values` is untyped in prom-client's JSON shape; narrow to what we assert on.
return (metric as unknown as { values: Array<{ labels: Record<string, string>; value: number }> })
.values;
}
describe('csamArchivePathFor', () => {
it('maps the two flag-governed types to stream/disk by the flag', () => {
expect(csamArchivePathFor('Image', true)).toBe('stream');
expect(csamArchivePathFor('Image', false)).toBe('disk');
expect(csamArchivePathFor('GeneratedImage', true)).toBe('stream');
expect(csamArchivePathFor('GeneratedImage', false)).toBe('disk');
});
it('🔴 maps every other type to `none` REGARDLESS of the flag', () => {
// The regression this pins: labelling a TrainingData report `stream` because
// the flag happened to be on would make the counter a record of the FLAG
// rather than of the code, which is the confusion it was added to end.
for (const type of ['TrainingData', 'ExternalLink'] as const) {
expect(csamArchivePathFor(type, true)).toBe('none');
expect(csamArchivePathFor(type, false)).toBe('none');
}
});
});
describe('REACHABLE_SERIES', () => {
it('holds exactly the 6 (path, type) pairs the code can produce', () => {
expect(REACHABLE_SERIES).toHaveLength(6);
const asKeys = REACHABLE_SERIES.map(({ path, type }) => `${type}:${path}`).sort();
expect(asKeys).toEqual(
[
'Image:stream',
'Image:disk',
'GeneratedImage:stream',
'GeneratedImage:disk',
'TrainingData:none',
'ExternalLink:none',
].sort()
);
});
it('🔴 excludes every combination the code CANNOT produce', () => {
// Seeding an impossible pair would put a permanent zero on screen that no code
// path can ever move — which reads as "this never happens" when it means "this
// cannot happen".
const produced = new Set(REACHABLE_SERIES.map(({ path, type }) => `${type}:${path}`));
const impossible = [
'ExternalLink:stream',
'ExternalLink:disk',
'TrainingData:stream',
'TrainingData:disk',
'Image:none',
'GeneratedImage:none',
];
for (const combo of impossible) expect(produced.has(combo)).toBe(false);
// The naive product is 3 x 4 = 12 pairs; exactly half are unreachable.
expect(produced.size + impossible.length).toBe(
CSAM_ARCHIVE_PATHS.length * CSAM_ARCHIVE_TYPES.length
);
});
});
describe('ensureRegisterCsamArchiveMetrics', () => {
it('🔴 seeds all 12 reachable series at 0, so a rare event is distinguishable from an unwired instrument', async () => {
const values = await seriesOf(freshRegistry());
expect(values).toHaveLength(REACHABLE_SERIES.length * CSAM_ARCHIVE_OUTCOMES.length);
expect(values).toHaveLength(12);
expect(values.every((v) => v.value === 0)).toBe(true);
for (const { path, type } of REACHABLE_SERIES) {
for (const outcome of CSAM_ARCHIVE_OUTCOMES) {
expect(
values.some(
(v) => v.labels.path === path && v.labels.type === type && v.labels.outcome === outcome
)
).toBe(true);
}
}
});
it('is idempotent — re-registering neither throws nor resets a counted value', async () => {
const reg = freshRegistry();
const { csamArchiveTotal } = ensureRegisterCsamArchiveMetrics(reg);
csamArchiveTotal.inc({ path: 'stream', type: 'Image', outcome: 'success' });
ensureRegisterCsamArchiveMetrics(reg);
ensureRegisterCsamArchiveMetrics(reg);
const v = (await seriesOf(reg)).find(
(x) =>
x.labels.path === 'stream' && x.labels.type === 'Image' && x.labels.outcome === 'success'
);
expect(v?.value).toBe(1);
// Still 12 — a second registration must not duplicate the series.
expect(await seriesOf(reg)).toHaveLength(12);
});
});
describe('recordCsamArchive', () => {
beforeEach(() => {
client.register.removeSingleMetric(NAME);
});
it('increments exactly the addressed series on the default registry', async () => {
recordCsamArchive('stream', 'Image', 'success');
recordCsamArchive('stream', 'Image', 'success');
recordCsamArchive('disk', 'GeneratedImage', 'error');
const values = await seriesOf(client.register);
const at = (path: string, type: string, outcome: string) =>
values.find(
(v) => v.labels.path === path && v.labels.type === type && v.labels.outcome === outcome
)?.value;
expect(at('stream', 'Image', 'success')).toBe(2);
expect(at('disk', 'GeneratedImage', 'error')).toBe(1);
// Everything else stays an observable 0 rather than vanishing.
expect(at('stream', 'Image', 'error')).toBe(0);
expect(at('none', 'TrainingData', 'success')).toBe(0);
});
it('🔴 DROPS an unknown label value rather than passing it through', async () => {
// The 12-series cardinality claim rests on this, not on the erased types: this
// runs on the jobs pool, where prom-client retains every distinct label set in
// the Node heap for the process lifetime.
// Seed first, exactly as /api/metrics does on every scrape. Without this the
// counter would not be registered at all here — the validity guard returns
// BEFORE `ensureRegister…`, so a run in which every call is dropped never
// creates the metric. That is correct behaviour, but it is not the production
// shape, and asserting against it would test the wrong thing.
ensureRegisterCsamArchiveMetrics();
recordCsamArchive('nonsense' as never, 'Image', 'success');
recordCsamArchive('stream', 'NotAType' as never, 'success');
recordCsamArchive('stream', 'Image', 'maybe' as never);
const values = await seriesOf(client.register);
expect(values).toHaveLength(12);
expect(values.every((v) => v.value === 0)).toBe(true);
});
it('🔴 is FAIL-SOFT — a poisoned registration cannot throw into the archive path', () => {
// Register the same name with a DIFFERENT labelset, which is what makes the
// unchecked `as` cast inside the module dangerous: `inc` on a mismatched
// labelset throws. The archive must survive it.
client.register.removeSingleMetric(NAME);
new client.Counter({
name: NAME,
help: 'poisoned: wrong labelset',
labelNames: ['totally', 'different'] as const,
registers: [client.register],
});
expect(() => recordCsamArchive('stream', 'Image', 'success')).not.toThrow();
});
});
+230
View File
@@ -0,0 +1,230 @@
// CSAM evidence-archive PATH counter.
//
// 🔴 WHY THIS EXISTS: the archiver has had two code paths since #4771 — stage the
// two large media zips on the container's `scratch` emptyDir, or stream them
// straight to object storage — selected per report by the Flipt flag
// `csam-archive-stream-upload`. Nothing recorded which one ran. After the flag
// went live, archives completed and answering "did the streaming path actually
// run?" was impossible: the job emitted no path-identifying log line, and every
// existing `civitai_csam_*` series is monitor-level — pending / scanned /
// oldest-age / last-success — so none of them says anything about HOW an archive
// completed. The flag being on is not evidence the flagged branch executed.
//
// 🔴 WHAT `path` MEANS, AND THE TWO ARMS DIFFER — read this before alerting on it.
// On an `outcome="success"` row it is what RAN: `archiveAndUpload` branches on the
// flag with no internal fallback, so reaching the success arm means that branch
// executed. On an `outcome="error"` row it is only the path SELECTED for this
// report. `archivePath` is resolved before any archiving begins, so a failure
// upstream of the media archive — the base-bundle upload, an image count, a
// scratch-volume WRITE inside the try (as distinct from the earlier mkdir, which
// throws before it and is not counted at all — see the help text) — records the
// selected path even though that branch never
// executed. Concretely: `{path="stream", outcome="error"}` does NOT establish that
// the streaming upload failed. Read it as "a report routed to streaming failed
// somewhere", then use the log line's report id to find out where.
//
// 🔴 WHAT IS GENUINELY BETTER THAN READING THE FLAG: the `none` mapping. The flag
// is read once per report but governs `archiveImages`/`archiveGeneratedImages`
// only. A `TrainingData` report downloads a file it did not build, and an
// `ExternalLink` report archives no media at all — for both, the flag is read and
// then has nothing to select. Labelling those `stream` because the flag happened
// to be on would make the series a record of the flag rather than of the code.
// For `Image`/`GeneratedImage` — the only two types where `stream` and `disk` can
// differ at all — an ERROR row's `path` is, in fact, exactly what the flag said.
//
// 🔴 ALERTING: use `max_over_time(...[window])`, NOT `rate()` / `increase()`.
// Same hazard as `generation-model-substitution.metrics.ts`, and worse here: CSAM
// archives are RARE, spread over a multi-pod jobs pool, so a pod that archives
// once creates its child at 1 and never touches it again. A `rate()` over that
// child is structurally 0, and an alert keyed on it silently never fires — the
// counter would look healthiest exactly when the thing it watches is happening.
//
// 🔴 CARDINALITY: 12 series, total, and every one of them is REACHABLE — see
// REACHABLE_SERIES below. The naive product is 3 x 4 x 2 = 24, but half of those
// are combinations the code cannot produce (`ExternalLink` + `stream`, `Image` +
// `none`, ...). Seeding the impossible ones would put permanent zeros on screen
// that no code path can ever move, which reads as "this never happens" when it
// means "this cannot happen". Deliberately NO report id, user id, or byte count:
// this runs on the jobs pool where prom-client retains every distinct label set
// in the Node heap for the process lifetime, and the per-report detail belongs in
// the log line the service already emits beside this.
import client, { type Counter, type Registry } from 'prom-client';
import type { CsamReportType } from '~/shared/utils/prisma/models';
export const CSAM_ARCHIVE_PATHS = ['stream', 'disk', 'none'] as const;
export type CsamArchivePath = (typeof CSAM_ARCHIVE_PATHS)[number];
export const CSAM_ARCHIVE_TYPES = [
'Image',
'GeneratedImage',
'TrainingData',
'ExternalLink',
] as const;
export type CsamArchiveType = (typeof CSAM_ARCHIVE_TYPES)[number];
export const CSAM_ARCHIVE_OUTCOMES = ['success', 'error'] as const;
export type CsamArchiveOutcome = (typeof CSAM_ARCHIVE_OUTCOMES)[number];
/**
* 🔴 COMPILE-TIME PIN AGAINST THE PRISMA ENUM — this is a GUARD, not documentation.
*
* `CSAM_ARCHIVE_TYPES` restates `CsamReportType` as a runtime array (a type alone
* cannot be iterated to seed series, and `isCsamArchiveType` needs values). A
* restatement drifts silently, and the drift is invisible at every other layer:
* a new report type would make `csamArchivePathFor` return `'none'` (no
* `FLAG_GOVERNED_TYPES` match), make the log line say "via the none path" for a
* type that may well build a media archive, and make `recordCsamArchive` DROP the
* call outright (`isCsamArchiveType` false) — so the new type would be absent from
* the counter with no error, no log and no failing test.
*
* This fails the BUILD in both directions instead. Adding a member to either union
* without the other is a type error here, at the one place that can see both.
*/
type AssertTrue<T extends true> = T;
// Unused BY CONSTRUCTION — the assertion IS the check, and it fires at compile time. There is
// nothing to reference it from; referencing it would not make it stronger.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
type _CsamArchiveTypesMatchPrisma = AssertTrue<
[CsamArchiveType] extends [CsamReportType]
? [CsamReportType] extends [CsamArchiveType]
? true
: false
: false
>;
/**
* Report types whose media archive is built by this code, and are therefore
* actually selected by the flag. Everything else gets `path: 'none'`.
*/
const FLAG_GOVERNED_TYPES: readonly CsamArchiveType[] = ['Image', 'GeneratedImage'];
/**
* The path a report of this type takes, given the flag value read for it.
*
* 🔴 This is the ONLY place the flag-to-path mapping is expressed, so the counter
* and the log line cannot disagree about what ran — they both call this.
*/
export function csamArchivePathFor(
type: CsamArchiveType,
streamArchivesToStorage: boolean
): CsamArchivePath {
if (!FLAG_GOVERNED_TYPES.includes(type)) return 'none';
return streamArchivesToStorage ? 'stream' : 'disk';
}
/**
* Every (path, type) pair the code above can actually produce. Derived from
* `csamArchivePathFor` rather than written out, so the two cannot drift: adding a
* type or a path to the unions extends this automatically, and a test asserts the
* count.
*/
export const REACHABLE_SERIES: ReadonlyArray<{ path: CsamArchivePath; type: CsamArchiveType }> =
CSAM_ARCHIVE_TYPES.flatMap((type) =>
[true, false]
.map((flag) => csamArchivePathFor(type, flag))
// A type that ignores the flag yields 'none' for both values; dedupe so it
// contributes one pair, not two identical ones.
.filter((path, i, all) => all.indexOf(path) === i)
.map((path) => ({ path, type }))
);
function isCsamArchivePath(v: unknown): v is CsamArchivePath {
return CSAM_ARCHIVE_PATHS.includes(v as CsamArchivePath);
}
function isCsamArchiveType(v: unknown): v is CsamArchiveType {
return CSAM_ARCHIVE_TYPES.includes(v as CsamArchiveType);
}
function isCsamArchiveOutcome(v: unknown): v is CsamArchiveOutcome {
return CSAM_ARCHIVE_OUTCOMES.includes(v as CsamArchiveOutcome);
}
/**
* Initialise all 12 reachable series to 0 at registration.
*
* 🔴 WHY THIS IS NOT COSMETIC, and why it matters more here than almost anywhere
* else in this repo: prom-client materialises a child only on its first `inc()`,
* and CSAM archives are rare. Without seeding, a pool that has not archived
* anything recently exposes NOTHING, and `civitai_csam_archive_total{path="stream"}`
* returns `no data` — indistinguishable from "the instrument was never wired",
* which is precisely the state this counter was added to escape. A real zero and
* an absent series must be tellable apart, and on a rare event the honest reading
* is a row of zeros for most of the counter's life.
*
* 🔴 SEEDING ALONE IS NOT ENOUGH. `ensureRegister…` has no caller on the archive
* path before the first archive, so the series would still be absent until an
* event happened. The other half is the side-effect call in
* `src/pages/api/metrics.ts`, which runs on the first scrape. Both halves are
* required; neither works alone.
*
* Idempotent: `getOrCreateCounter` returns the existing counter and `inc(…, 0)` is
* a no-op on an already-materialised series, so calling this per request cannot
* reset or double-count.
*/
function seedAllSeries(counter: Counter<string>): void {
for (const { path, type } of REACHABLE_SERIES) {
for (const outcome of CSAM_ARCHIVE_OUTCOMES) counter.inc({ path, type, outcome }, 0);
}
}
function getOrCreateCounter(
reg: Registry,
name: string,
help: string,
labelNames: string[]
): Counter<string> {
const existing = reg.getSingleMetric(name) as Counter<string> | undefined;
if (existing) return existing;
return new client.Counter({ name, help, labelNames, registers: [reg] });
}
/**
* Idempotent: safe to call on every request. Returns the counter from the default
* registry that /api/metrics scrapes.
*/
export function ensureRegisterCsamArchiveMetrics(reg: Registry = client.register): {
csamArchiveTotal: Counter<string>;
} {
const csamArchiveTotal = getOrCreateCounter(
reg,
'civitai_csam_archive_total',
'CSAM evidence-archive attempts that reached the archive stage, by the media-archive path selected for them. ' +
'NOT every terminal state: a report with no reported user is stamped archived and returns BEFORE the flag is read, and the scratch-volume MKDIR (not the later writes, which are counted) throws before the try block, so neither is counted here. Both are still recorded elsewhere — the first as details.archiveSkipped on the row, the second as a csam-report log with subType=archive-data carrying the errno, plus the subType=archive-path line that attributes it to a report and a path. ' +
'On outcome=success, path is what RAN. On outcome=error it is only the path SELECTED: the failure may be upstream of the media archive, so {path="stream",outcome="error"} does not establish that streaming failed. ' +
'path (stream = the two large media zips were streamed straight to object storage, the #4771 path behind the csam-archive-stream-upload flag; ' +
'disk = they were staged on the container scratch emptyDir first, the long-standing path and the flag-off rollback; ' +
'none = this report type builds no large media archive here, so the flag selected nothing — TrainingData downloads a prebuilt file and ExternalLink archives only base user data). ' +
'type = CsamReport.type. outcome (success = archivedAt was set; error = the per-report archive threw). ' +
'RARE EVENT: alert with max_over_time(), never rate()/increase() — a pod that archives once sets its child to 1 and never moves it again.',
['path', 'type', 'outcome']
);
seedAllSeries(csamArchiveTotal);
return { csamArchiveTotal };
}
/**
* Fail-soft emit of one terminal archive attempt.
*
* 🔴 TOTAL, like every emitter in this directory. This instruments the CSAM
* evidence path, which is a legal-reporting obligation: a metrics error
* (registry collision, label mismatch) must never propagate and turn an archive
* that actually succeeded into a failed one. Observability must not be able to
* cause the outage it is watching for.
*/
export function recordCsamArchive(
path: CsamArchivePath,
type: CsamArchiveType,
outcome: CsamArchiveOutcome
): void {
try {
// 🔴 The cardinality bound rests HERE, on code, not on the erased types above.
// An unknown value is DROPPED rather than passed through or relabelled to a
// plausible default — either would make the "12 series" claim a wish.
if (!isCsamArchivePath(path) || !isCsamArchiveType(type) || !isCsamArchiveOutcome(outcome))
return;
const { csamArchiveTotal } = ensureRegisterCsamArchiveMetrics();
csamArchiveTotal.inc({ path, type, outcome });
} catch {
/* instrument-only — never let a metrics error touch the CSAM archive path */
}
}
@@ -184,9 +184,21 @@ vi.mock('~/server/flipt/client', async (importOriginal) => ({
isFlipt: (...args: unknown[]) => mockIsFlipt(...args),
}));
// 🔴 SPY ONLY ON THE EMITTER, keeping `csamArchivePathFor` REAL. Replacing the whole module
// would make the seam tests below assert against a mapping this file invented, which is the
// one thing they must not do — the service and the counter have to agree about the REAL
// mapping or the guard is vacuous.
vi.mock('~/server/metrics/csam-archive.metrics', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
recordCsamArchive: (...args: unknown[]) => {
recordedArchiveCalls.push(args as [string, string, string]);
},
}));
let mockFetchBlob: (...args: unknown[]) => unknown = () => null;
let mockGetConsumerStrikes: (...args: unknown[]) => unknown = async () => [];
let mockIsFlipt: (...args: unknown[]) => unknown = async () => false;
let recordedArchiveCalls: Array<[string, string, string]> = [];
import { archiveCsamDataForReport } from '~/server/services/csam.service-new';
import { dbMock } from '~/__tests__/mocks/db.mock';
@@ -400,6 +412,7 @@ async function settleOrHang(reportArg: unknown) {
beforeEach(() => {
storedObjects.clear();
pendingUploads.clear();
recordedArchiveCalls = [];
mockFetchBlob = async (url: unknown) => {
const match = /image-uuid-(\d+)/.exec(String(url));
if (!match) return null;
@@ -754,3 +767,89 @@ describe('csam archive streaming upload', () => {
expect((rejection as Error).message).not.toMatch(/failed to upload/);
}, 20_000);
});
// ===============================================================================================
// THE SEAM: does the service actually CALL the counter?
//
// 🔴 WHY THIS BLOCK EXISTS. `csam-archive.metrics.test.ts` drives the counter module directly and
// pins its cardinality, seeding and fail-soft behaviour — all real, and all blind to the only
// question that decides whether any of it reaches production: whether `archiveCsamDataForReport`
// invokes it at all — because every test that loads the service asserted on archives and blobs,
// and every test that loads the counter called it itself. Two surfaces, each hermetically tested,
// with the defect living in the seam neither one owns.
//
// MEASURED, and stated with its exact scope because a bare pass count here is unreproducible:
// with both `recordCsamArchive(...)` lines deleted from the service, running
// `csam-archive.metrics.test.ts` + `csam-archive-stream-upload.test.ts` +
// `csam-archive-backpressure.test.ts` leaves all 37 NON-SEAM tests in those three files
// green, and ONLY the four cases below go red. (A wider file set gives a different total; the
// number that means anything is "every pre-existing test stayed green", not the total.)
//
// So these cases assert a RELATIONSHIP — service → emitter — not a component. They would also
// catch the two call sites having their `success`/`error` arguments swapped, which no
// module-level test can see.
// ===============================================================================================
describe('the service→counter seam', () => {
it('🔴 records the STREAM path on a successful flag-ON archive', async () => {
mockIsFlipt = async (flag: unknown) => flag === FLIPT_FEATURE_FLAGS.CSAM_ARCHIVE_STREAM_UPLOAD;
await archiveCsamDataForReport(report as never);
// Positive control: the archive must actually have completed, or the assertion below is
// about a run that never reached the success arm.
expect(
storedObjects.get('images.zip'),
'the archive stored no images.zip — this run never reached the success arm'
).toBeDefined();
expect(recordedArchiveCalls).toEqual([['stream', 'Image', 'success']]);
}, 20_000);
it('🔴 records the DISK path when the flag is OFF — the two arms are not interchangeable', async () => {
mockIsFlipt = async () => false;
await archiveCsamDataForReport(report as never);
expect(
storedObjects.get('images.zip'),
'the archive stored no images.zip — this run never reached the success arm'
).toBeDefined();
expect(recordedArchiveCalls).toEqual([['disk', 'Image', 'success']]);
}, 20_000);
it('🔴 records outcome=error when the archive throws, and does NOT record success', async () => {
const failure = new Error('blob fetch exploded');
mockFetchBlob = async (url: unknown) => {
const match = /image-uuid-(\d+)/.exec(String(url));
if (!match) return null;
if (Number(match[1]) === 11) throw failure;
return fakeBlob(Number(match[1]));
};
mockIsFlipt = async (flag: unknown) => flag === FLIPT_FEATURE_FLAGS.CSAM_ARCHIVE_STREAM_UPLOAD;
const rejection = await archiveCsamDataForReport(report as never).then(
() => undefined,
(e: unknown) => e
);
// Positive control, same reasoning as the rejection test above: if nothing failed, the
// outcome assertion is trivially satisfiable by a run that simply succeeded.
expect(
rejection,
'the archive did not fail — the fault injection is wired to nothing'
).toBeDefined();
expect(recordedArchiveCalls).toEqual([['stream', 'Image', 'error']]);
// Explicit, because a swapped pair at the two call sites would still produce ONE call.
expect(recordedArchiveCalls.some(([, , outcome]) => outcome === 'success')).toBe(false);
}, 20_000);
it('🔴 records path=none for a type the flag does not govern', async () => {
// ExternalLink archives only base user data, so the flag selects nothing. Flag deliberately
// ON: if the service passed the flag straight through instead of the resolved path, this
// would read "stream".
mockIsFlipt = async (flag: unknown) => flag === FLIPT_FEATURE_FLAGS.CSAM_ARCHIVE_STREAM_UPLOAD;
await archiveCsamDataForReport({ ...report, type: 'ExternalLink' } as never);
expect(recordedArchiveCalls).toEqual([['none', 'ExternalLink', 'success']]);
}, 20_000);
});
+67 -4
View File
@@ -53,6 +53,7 @@ import type { JsonReplacer } from '~/server/utils/json-stream-helpers';
import { writeJsonObject } from '~/server/utils/json-stream-helpers';
import { getConsumerStrikes } from '~/server/http/orchestrator/flagged-consumers';
import { logToAxiom } from '~/server/logging/client';
import { csamArchivePathFor, recordCsamArchive } from '~/server/metrics/csam-archive.metrics';
import { trimNonAlphanumeric } from '~/utils/string-helpers';
const cybertipClient = new Client({
@@ -1504,10 +1505,6 @@ export async function archiveCsamDataForReport(data: CsamReportProps) {
trainingData: `${baseDir}/training-data/${data.id}`,
};
for (const dir of Object.values(reportDirs)) {
createDir(dir);
}
/**
* Whether the two large media archives are streamed straight to object storage instead of being
* staged on the container's scratch volume first.
@@ -1527,6 +1524,55 @@ export async function archiveCsamDataForReport(data: CsamReportProps) {
*/
const streamArchivesToStorage = await isFlipt(FLIPT_FEATURE_FLAGS.CSAM_ARCHIVE_STREAM_UPLOAD);
/**
* The media-archive path SELECTED for this report.
*
* 🔴 Not the same thing as the flag value: the flag governs
* `archiveImages`/`archiveGeneratedImages` only, so a `TrainingData` or `ExternalLink`
* report resolves to `none` however the flag is set. Derived in one place so the log
* line and the counter cannot disagree about which path this report was routed to.
*/
const archivePath = csamArchivePathFor(report.type, streamArchivesToStorage);
/**
* 🔴 EMITTED AT THE DECISION, AND BEFORE `createDir` — both deliberately.
*
* At the decision, because an archive that dies partway never reaches either counter
* call, so a completion-only record would be silent for exactly the failures worth
* investigating. This line is the one that survives a killed container.
*
* Before `createDir`, because that loop writes to the scratch volume and sits OUTSIDE
* the try block below — so a failure there (the disk-pressure class this whole arc was
* about) throws past both counter calls.
*
* 🔴 IT WAS NOT INVISIBLE BEFORE, AND SAYING SO WOULD BE WRONG: the caller already
* catches and logs it — `process-csam.ts`'s per-report `catch` emits
* `subType: 'archive-data'` carrying `errorMessage(e)`, which for the `mkdirSync` throw
* is the real errno (`ENOSPC: … mkdir '<path>'`). What that record does NOT carry is any
* of `reportId`, `reportType` or `archivePath`. So what emitting first actually buys is
* ATTRIBUTION — which report, and which path it was routed to — not visibility. The
* counter still cannot see it either way, which the counter's own help text states.
*
* 🔴 Do NOT "fix" that by moving the `createDir` loop inside the try: `removeDir` is
* `fs.rmSync(dir, { recursive: true })` with no `force`, so it throws `ENOENT` on a
* directory that was never created — and the catch's cleanup loop would then throw from
* inside the catch. Documenting the blind spot is the cheaper correct answer.
*/
logToAxiom({
name: 'csam-report',
type: 'info',
subType: 'archive-path',
message: `archiving report ${report.id} (${report.type}) via the ${archivePath} path`,
reportId: report.id,
reportType: report.type,
archivePath,
streamArchivesToStorage,
});
for (const dir of Object.values(reportDirs)) {
createDir(dir);
}
/**
* A fresh keyset-paged scan of every image the reported user owns.
*
@@ -1578,6 +1624,8 @@ export async function archiveCsamDataForReport(data: CsamReportProps) {
},
});
recordCsamArchive(archivePath, report.type, 'success');
for (const dir of Object.values(reportDirs)) removeDir(dir);
} catch (e) {
console.log(e);
@@ -1592,6 +1640,21 @@ export async function archiveCsamDataForReport(data: CsamReportProps) {
});
}
}
// 🔴 TWO THINGS THIS ROW DOES NOT MEAN, both easy to misread:
//
// (a) `path` here is the path SELECTED, not the path that failed. `archivePath` was
// resolved before any archiving began, so a failure upstream of the media archive
// — the base bundle, an image count — still records it. `{path="stream",
// outcome="error"}` does NOT establish that the streaming upload broke; use the
// log line's report id to find where it actually died.
// (b) `error` means THIS CALL THREW, not that the report is unarchived: the
// `training data not found` branch above stamps `archivedAt` and then rethrows,
// so that benign case lands here while the row reads archived. Left as one
// outcome rather than three — it is confined to a type the flag does not govern,
// the `type` label makes it self-explanatory, and a third value would widen every
// series in the counter to describe one known-benign branch.
recordCsamArchive(archivePath, report.type, 'error');
for (const dir of Object.values(reportDirs)) removeDir(dir);
throw e;
}