From fa5ed3db5251ba71f7b862d7ef52f2f0e5618ad4 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Mon, 16 Sep 2024 02:01:23 +0000 Subject: [PATCH] chore: make CoreOwnership core IDs required This change: - Marks all `CoreOwnership` core IDs `required` in the proto - Gives each core ID a minimum length in the schema - Validates this length when decoding - Adds a test for the above --- proto/coreOwnership/v1.proto | 12 ++++++------ schema/coreOwnership/v1.json | 15 ++++++++++----- src/lib/decode-conversions.ts | 24 ++++++++++++++++++++++-- src/lib/utils.ts | 4 ++++ test/fixtures/bad-docs.js | 19 +++++++++++++++++++ 5 files changed, 61 insertions(+), 13 deletions(-) diff --git a/proto/coreOwnership/v1.proto b/proto/coreOwnership/v1.proto index c5561849..4a78354d 100644 --- a/proto/coreOwnership/v1.proto +++ b/proto/coreOwnership/v1.proto @@ -13,12 +13,12 @@ message CoreOwnership_1 { Common_1 common = 1; - bytes authCoreId = 5; - bytes configCoreId = 6; - bytes dataCoreId = 7; - bytes blobCoreId = 8; - bytes blobIndexCoreId = 9; - CoreSignatures coreSignatures = 10; + bytes authCoreId = 5 [(required) = true]; + bytes configCoreId = 6 [(required) = true]; + bytes dataCoreId = 7 [(required) = true]; + bytes blobCoreId = 8 [(required) = true]; + bytes blobIndexCoreId = 9 [(required) = true]; + CoreSignatures coreSignatures = 10 [(required) = true]; bytes identitySignature = 11; message CoreSignatures { diff --git a/schema/coreOwnership/v1.json b/schema/coreOwnership/v1.json index d450d5d4..659c0e37 100644 --- a/schema/coreOwnership/v1.json +++ b/schema/coreOwnership/v1.json @@ -11,23 +11,28 @@ }, "authCoreId": { "type": "string", - "description": "Hex-encoded key of auth store writer core" + "description": "Hex-encoded key of auth store writer core", + "minLength": 1 }, "configCoreId": { "type": "string", - "description": "Hex-encoded key of config store writer core" + "description": "Hex-encoded key of config store writer core", + "minLength": 1 }, "dataCoreId": { "type": "string", - "description": "Hex-encoded key of data store writer core" + "description": "Hex-encoded key of data store writer core", + "minLength": 1 }, "blobCoreId": { "type": "string", - "description": "Hex-encoded key of blob store writer core" + "description": "Hex-encoded key of blob store writer core", + "minLength": 1 }, "blobIndexCoreId": { "type": "string", - "description": "Hex-encoded key of blobIndex store writer core" + "description": "Hex-encoded key of blobIndex store writer core", + "minLength": 1 } }, "required": [ diff --git a/src/lib/decode-conversions.ts b/src/lib/decode-conversions.ts index 3b68795f..3fca2d4d 100644 --- a/src/lib/decode-conversions.ts +++ b/src/lib/decode-conversions.ts @@ -20,7 +20,6 @@ import { type JsonTagValue, type MapeoDocDecode, } from '../types.js' -import { ExhaustivenessError, VersionIdObject, getVersionId } from './utils.js' import type { Icon, Observation, Track } from '../index.js' import type { Observation_1_Attachment, @@ -31,6 +30,12 @@ import type { Track_1_Position } from '../proto/track/v1.js' import { ProjectSettings_1_ConfigMetadata } from '../proto/projectSettings/v1.js' import { ProjectSettings } from '../schema/projectSettings.js' import type { Position } from '../schema/observation.js' +import { + assert, + ExhaustivenessError, + getVersionId, + VersionIdObject, +} from './utils.js' /** Function type for converting a protobuf type of any version for a particular * schema name, and returning the most recent JSONSchema type */ @@ -39,6 +44,14 @@ type ConvertFunction = ( versionObj: VersionIdObject ) => FilterBySchemaName +function ensure( + condition: unknown, + objectName: string, + propertyName: string +): asserts condition { + assert(condition, `${objectName} missing required property ${propertyName}`) +} + export const convertProjectSettings: ConvertFunction<'projectSettings'> = ( message, versionObj @@ -223,8 +236,15 @@ export const convertCoreOwnership: ConvertFunction<'coreOwnership'> = ( dataCoreId, blobCoreId, blobIndexCoreId, + coreSignatures, ...rest } = message + ensure(coreSignatures, 'coreOwnership', 'coreSignatures') + ensure(authCoreId.byteLength, 'coreOwnership', 'authCoreId') + ensure(configCoreId.byteLength, 'coreOwnership', 'configCoreId') + ensure(dataCoreId.byteLength, 'coreOwnership', 'dataCoreId') + ensure(blobCoreId.byteLength, 'coreOwnership', 'blobCoreId') + ensure(blobIndexCoreId.byteLength, 'coreOwnership', 'blobIndexCoreId') const jsonSchemaCommon = convertCommon(common, versionObj) return { ...jsonSchemaCommon, @@ -234,7 +254,7 @@ export const convertCoreOwnership: ConvertFunction<'coreOwnership'> = ( dataCoreId: dataCoreId.toString('hex'), blobCoreId: blobCoreId.toString('hex'), blobIndexCoreId: blobIndexCoreId.toString('hex'), - coreSignatures: message.coreSignatures, + coreSignatures, } } diff --git a/src/lib/utils.ts b/src/lib/utils.ts index d7f8891c..a68d06ed 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -12,6 +12,10 @@ export function getOwn( return Object.hasOwn(obj, key) ? obj[key] : undefined } +export function assert(condition: unknown, message: string): asserts condition { + if (!condition) throw new Error(message) +} + export class ExhaustivenessError extends Error { constructor(value: never) { super(`Exhaustiveness check failed. ${value} should be impossible`) diff --git a/test/fixtures/bad-docs.js b/test/fixtures/bad-docs.js index eac3cabd..f5693713 100644 --- a/test/fixtures/bad-docs.js +++ b/test/fixtures/bad-docs.js @@ -62,6 +62,25 @@ export const badDocs = [ deleted: false, }, }, + { + text: 'core ownership with empty core ID', + /** @type {import('../../dist/index.js').CoreOwnership} */ + doc: { + docId: cachedValues.docId, + versionId: cachedValues.versionId, + originalVersionId: cachedValues.versionId, + schemaName: 'coreOwnership', + createdAt: cachedValues.createdAt, + updatedAt: cachedValues.updatedAt, + links: [], + deleted: false, + authCoreId: cachedValues.coreId, + configCoreId: '', + dataCoreId: cachedValues.coreId, + blobCoreId: cachedValues.coreId, + blobIndexCoreId: cachedValues.coreId, + }, + }, { text: 'role doc with empty roleId', /** @type {import('../../dist/index.js').Role} */