mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
fix(licensing): reject SellMerge at the upsert contract, not just in the UI
Removing the checkbox removed the affordance, not the endpoint. licensingSchema derived its member set from CommercialUse, which this PR extended, so the write contract widened by itself: any signed-in owner posting model.upsert from the site origin could persist the label, and upsertModel spreads ...data straight into create and update. That is a lower bar than the admin backfill already documented as a writer -- a session cookie rather than a token. One such row throws on READ for a whole result set on every pod still running the previous build, across three typed selects including the public /api/v1/models. Three review lanes found this independently. Rejected at runtime rather than by narrowing the enum: narrowing it also narrows the inferred type, which broke six unrelated files including ModelWizard. The refine leaves the type CommercialUse[] and costs no consumer a change. Tomorrow's PR deletes the refine alongside the two defaults and the option. Controlled: removing the refine, inverting some->every, and emptying the withheld list each redden. The new case exercises the contract rather than reading the source, because a text guard cannot tell a live rejection from one that was written and never wired in, and it carries a positive control -- the same payload without SellMerge must parse, or the rejection proves nothing about SellMerge. Also closes three evasions of the source guard found this round: both captures stopped at a ], so .concat() after the literal or a .push() on the next line put the value back invisibly; and the guard reddened on an innocent comment reword, because the explanatory comment sits inside the captured region. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+10
-8
@@ -1,12 +1,14 @@
|
||||
-- Apply BEFORE the deploy, not after, and it is safe in both directions: the build that ships with
|
||||
-- this migration KNOWS 'SellMerge' and WRITES none. The schema @default and the upload form's
|
||||
-- default set are deliberately still the four-value array, so no row can carry the label while
|
||||
-- pods are mixed, and no pod on either build can fail to read one.
|
||||
-- Apply BEFORE the deploy. The build that ships with this migration knows 'SellMerge' and no
|
||||
-- PRODUCT path writes it: the schema @default and the upload form's default set are both back to
|
||||
-- the four-value array, and the form offers no SellMerge option. What can still write it is a
|
||||
-- hand-built upsertModel payload -- the zod enum accepts the member -- and the admin backfill
|
||||
-- named below. Neither happens by accident during a deploy, and both require naming a value the
|
||||
-- caller had to read the enum to know exists.
|
||||
--
|
||||
-- That is the expand half of expand/contract, and it is why there is no rolling-deploy window here
|
||||
-- at all. Turning the write paths on is a separate PR, shipped once every pod knows the label;
|
||||
-- until it lands, a creator taking defaults gets the four-value set and the backfill has not run,
|
||||
-- so both are one state rather than two.
|
||||
-- That is the expand half of expand/contract. It matters because Prisma deserializes the enum for
|
||||
-- a whole result set, so a row carrying a label a previous-build pod does not know throws on READ,
|
||||
-- not on the write. Turning the product paths on is a separate PR, shipped once every pod knows
|
||||
-- the label.
|
||||
--
|
||||
-- The order matters the other way round too: applying this first means the defaults-on PR needs no
|
||||
-- coordination with a migration.
|
||||
|
||||
@@ -121,8 +121,8 @@ const commercialUseOptions: Array<{ value: CommercialUse; label: string }> = [
|
||||
{ value: CommercialUse.Rent, label: 'Use on other generation services' },
|
||||
{ value: CommercialUse.Sell, label: 'Sell this model' },
|
||||
// SellMerge is deliberately absent until every pod knows the label -- see the decision test in
|
||||
// license-sell-merge-split.test.ts. An option a creator can tick writes the row a pod on the
|
||||
// previous build cannot read, which is the passive default path with a person behind it.
|
||||
// license-sell-merge-split.test.ts. An option a creator can tick writes the same row the default
|
||||
// used to write, with a creator behind it, and a previous-build pod cannot read it.
|
||||
];
|
||||
|
||||
// 'tags' is deliberately absent: the field is named `tagsOnModels`, so the watch effect below
|
||||
|
||||
@@ -33,13 +33,30 @@ import { postgresSlugify } from '~/utils/string-helpers';
|
||||
import { booleanString, commaDelimitedNumberArray } from '~/utils/zod-helpers';
|
||||
import type { ProfanityEvaluation } from '~/libs/profanity-simple';
|
||||
|
||||
// Rejected at runtime rather than removed from the enum, so the inferred type stays CommercialUse[]
|
||||
// and no consumer has to change for one day. SellMerge is held back until every pod knows the
|
||||
// label: a row carrying it throws on READ for a whole result set on any pod still running the
|
||||
// previous build, and this endpoint is reachable by any signed-in owner, not only through the
|
||||
// upload form -- removing the checkbox removed the affordance, not the endpoint.
|
||||
// The PR that turns the write paths on deletes this refine alongside the two defaults and the option.
|
||||
const WITHHELD_COMMERCIAL_USE: CommercialUse[] = [CommercialUse.SellMerge];
|
||||
|
||||
const licensingSchema = z.object({
|
||||
allowNoCredit: z.boolean().optional(),
|
||||
allowCommercialUse: z
|
||||
.preprocess((val) => {
|
||||
if (!val) return undefined;
|
||||
return Array.isArray(val) ? val : [val];
|
||||
}, z.enum(CommercialUse).array().optional())
|
||||
.preprocess(
|
||||
(val) => {
|
||||
if (!val) return undefined;
|
||||
return Array.isArray(val) ? val : [val];
|
||||
},
|
||||
z
|
||||
.enum(CommercialUse)
|
||||
.array()
|
||||
.refine((values) => !values.some((v) => WITHHELD_COMMERCIAL_USE.includes(v)), {
|
||||
message: 'That commercial use permission is not available yet',
|
||||
})
|
||||
.optional()
|
||||
)
|
||||
.optional(),
|
||||
allowDerivatives: z.boolean().optional(),
|
||||
allowDifferentLicense: z.boolean().optional(),
|
||||
|
||||
@@ -270,15 +270,21 @@ describe('sell / sell-merge split in the generated licence', () => {
|
||||
* `slice(0, -1)` both leave every negative assertion above passing for free.
|
||||
*/
|
||||
/**
|
||||
* DELIBERATE, AND NOT AN OVERSIGHT: the build that ships the SellMerge label writes none of it.
|
||||
* Both default sets stay four-valued until every pod knows the label, which is what makes the
|
||||
* migration safe to apply ahead of the deploy and leaves no rolling-deploy window -- a row
|
||||
* carrying a label an old pod cannot decode throws on READ, for the whole result set.
|
||||
* DELIBERATE, AND NOT AN OVERSIGHT: no product path in this build writes the SellMerge label --
|
||||
* both default sets stay four-valued and the form offers no option -- until every pod knows it.
|
||||
* A hand-built API payload still can; the migration file carries that and the reason.
|
||||
*
|
||||
* The PR that turns the write paths on inverts this case. If you are here because it looks like
|
||||
* the feature was half-shipped: it was, on purpose, for one day.
|
||||
*/
|
||||
it('ships the SellMerge label with no way to write it: defaults and the option list', () => {
|
||||
it('the two product write paths for SellMerge are off: the defaults and the option list', () => {
|
||||
const members = (capture: string) =>
|
||||
capture
|
||||
.replace(/\/\/[^\n]*/g, '')
|
||||
.split(',')
|
||||
.map((s) => s.trim().replace(/^CommercialUse\./, ''))
|
||||
.filter(Boolean);
|
||||
|
||||
const schema = readFileSync(
|
||||
join(__dirname, '../../../../packages/civitai-db-schema/prisma/schema.full.prisma'),
|
||||
'utf8'
|
||||
@@ -286,26 +292,72 @@ describe('sell / sell-merge split in the generated licence', () => {
|
||||
const modelDefault = schema.match(
|
||||
/allowCommercialUse\s+CommercialUse\[\]\s+@default\(\[(.*)\]\)/
|
||||
);
|
||||
expect(modelDefault?.[1]).toBe('Image, RentCivit, Rent, Sell');
|
||||
expect(modelDefault).not.toBeNull();
|
||||
// Set rather than exact string: `prisma format` may reorder or respace this line, and a guard
|
||||
// that reddens on formatting is one the third person to hit it deletes.
|
||||
expect(members(modelDefault?.[1] ?? '').sort()).toEqual(
|
||||
['Image', 'Rent', 'RentCivit', 'Sell'].sort()
|
||||
);
|
||||
|
||||
const form = readFileSync(
|
||||
join(__dirname, '../../../components/Resource/Forms/ModelUpsertForm.tsx'),
|
||||
'utf8'
|
||||
);
|
||||
// Anchored on the statement's end, not on the first `]`: a capture stopping at the bracket
|
||||
// cannot see `?? [...].concat(SellMerge)`, which puts the value back with the literal intact.
|
||||
const formDefault = form.match(
|
||||
/allowCommercialUse: model\?\.allowCommercialUse \?\? \[([^\]]*)\]/
|
||||
/allowCommercialUse: model\?\.allowCommercialUse \?\? \[([\s\S]*?)\],\n/
|
||||
);
|
||||
expect(formDefault).not.toBeNull();
|
||||
expect(formDefault?.[1]).not.toContain('SellMerge');
|
||||
// Positive control: the array the match found is the real one, not an empty capture.
|
||||
expect(formDefault?.[1]).toContain('CommercialUse.Sell');
|
||||
// Asserted as a SET, so dropping members to hide one reddens too.
|
||||
expect(members(formDefault?.[1] ?? '').sort()).toEqual(
|
||||
['Image', 'Rent', 'RentCivit', 'Sell'].sort()
|
||||
);
|
||||
|
||||
// The default is the passive write path. The checkbox is the active one, and removing only the
|
||||
// first leaves a creator able to write the row a previous-build pod cannot read.
|
||||
const options = form.match(/const commercialUseOptions[^=]*= \[([\s\S]*?)\n\];/);
|
||||
const options = form.match(/const commercialUseOptions[\s\S]*?=\s*\[([\s\S]*?)\n\];/);
|
||||
expect(options).not.toBeNull();
|
||||
expect(options?.[1]).not.toContain('CommercialUse.SellMerge');
|
||||
expect(options?.[1]).toContain('CommercialUse.Sell,');
|
||||
// Value position, not a bare substring: the comment standing where the option used to be names
|
||||
// SellMerge, and a reader writing it as `CommercialUse.SellMerge` there must not redden this.
|
||||
expect(options?.[1]).not.toMatch(/value:\s*CommercialUse\.SellMerge/);
|
||||
expect(options?.[1]).toMatch(/value:\s*CommercialUse\.Sell\b/);
|
||||
|
||||
// The option list is also reachable by mutation after its literal, which no capture above sees.
|
||||
expect(form).not.toMatch(/commercialUseOptions\s*\.\s*(push|splice|concat|unshift)/);
|
||||
|
||||
// The third door, and the only one that opened by itself: the upsert contract derived its
|
||||
// member set from the enum, so extending the enum widened what a signed-in owner may POST.
|
||||
// Removing the checkbox removed the affordance, not the endpoint.
|
||||
const schemaSrc = readFileSync(join(__dirname, '../../schema/model.schema.ts'), 'utf8');
|
||||
const withheld = schemaSrc.match(/const WITHHELD_COMMERCIAL_USE[^=]*=\s*\[([\s\S]*?)\]/);
|
||||
expect(withheld).not.toBeNull();
|
||||
expect(members(withheld?.[1] ?? '')).toEqual(['SellMerge']);
|
||||
// ...and the list must actually be applied, or it is decoration.
|
||||
expect(schemaSrc).toMatch(/WITHHELD_COMMERCIAL_USE\.includes/);
|
||||
});
|
||||
|
||||
/**
|
||||
* The list above is a source-text assertion; this one runs the contract. A text guard cannot tell
|
||||
* a rejection from a refine that was written and never wired in.
|
||||
*/
|
||||
it('the upsert contract rejects a withheld permission and accepts the rest', async () => {
|
||||
const { modelUpsertSchema } = await import('~/server/schema/model.schema');
|
||||
const base = { name: 'x', type: 'Checkpoint', status: 'Draft', uploadType: 'Created' };
|
||||
|
||||
const withheld = modelUpsertSchema.safeParse({
|
||||
...base,
|
||||
allowCommercialUse: [CommercialUse.Sell, CommercialUse.SellMerge],
|
||||
});
|
||||
expect(withheld.success).toBe(false);
|
||||
|
||||
// Positive control: the same payload without the withheld member must pass, or the rejection
|
||||
// above proves nothing about SellMerge -- it could be any other field failing.
|
||||
const allowed = modelUpsertSchema.safeParse({
|
||||
...base,
|
||||
allowCommercialUse: [CommercialUse.Sell],
|
||||
});
|
||||
expect(allowed.success).toBe(true);
|
||||
});
|
||||
|
||||
it('the slicers refuse a document they cannot find their anchor in', () => {
|
||||
|
||||
Reference in New Issue
Block a user