helpers for rkey and tid syntax; validate rkey at record creation time (

bluesky-social#1738)

* syntax: fix jest config displayName

* syntax: TID validation

* syntax: add recordkey validation

* pds: verify rkey syntax at record creation time

---------

Co-authored-by: dholms <[email protected]>

interop-test-files/syntax/tid_syntax_invalid.txt

@@ -0,0 +1,15 @@
+
+# not base32
+3jzfcijpj2z21
+0000000000000
+
+# too long/short
+3jzfcijpj2z2aa
+3jzfcijpj2z2
+
+# old dashes syntax not actually supported (TTTT-TTT-TTTT-CC)
+3jzf-cij-pj2z-2a
+
+# high bit can't be high
+zzzzzzzzzzzzz
+kjzfcijpj2z2a

interop-test-files/syntax/tid_syntax_valid.txt

@@ -0,0 +1,6 @@
+# 13 digits
+# 234567abcdefghijklmnopqrstuvwxyz
+
+3jzfcijpj2z2a
+7777777777777
+3zzzzzzzzzzzz

packages/pds/src/api/com/atproto/repo/createRecord.ts

@@ -1,5 +1,6 @@
 import { CID } from 'multiformats/cid'
 import { InvalidRequestError, AuthRequiredError } from '@atproto/xrpc-server'
+import { InvalidRecordKeyError } from '@atproto/syntax'
 import { prepareCreate } from '../../../../repo'
 import { Server } from '../../../../lexicon'
 import {
@@ -59,6 +60,9 @@ export default function (server: Server, ctx: AppContext) {
         if (err instanceof InvalidRecordError) {
           throw new InvalidRequestError(err.message)
         }
+        if (err instanceof InvalidRecordKeyError) {
+          throw new InvalidRequestError(err.message)
+        }
         throw err
       }
 

packages/pds/src/repo/prepare.ts

@@ -1,5 +1,9 @@
 import { CID } from 'multiformats/cid'
-import { AtUri, ensureValidDatetime } from '@atproto/syntax'
+import {
+  AtUri,
+  ensureValidRecordKey,
+  ensureValidDatetime,
+} from '@atproto/syntax'
 import { MINUTE, TID, dataToCborBlock } from '@atproto/common'
 import {
   LexiconDefNotFoundError,
@@ -188,6 +192,8 @@ export const prepareCreate = async (opts: {
   }
 
   const rkey = opts.rkey || nextRkey.toString()
+  // @TODO: validate against Lexicon record 'key' type, not just overall recordkey syntax
+  ensureValidRecordKey(rkey)
   assertNoExplicitSlurs(rkey, record)
   return {
     action: WriteOpAction.Create,

packages/pds/tests/crud.test.ts

@@ -567,6 +567,22 @@ describe('crud operations', () => {
     )
   })
 
+  it('requires valid rkey', async () => {
+    await expect(
+      aliceAgent.api.com.atproto.repo.createRecord({
+        repo: alice.did,
+        collection: 'app.bsky.feed.generator',
+        record: {
+          $type: 'app.bsky.feed.generator',
+          did: 'did:web:dummy.example.com',
+          displayName: 'dummy',
+          createdAt: new Date().toISOString(),
+        },
+        rkey: '..',
+      }),
+    ).rejects.toThrow('record key can not be "." or ".."')
+  })
+
   it('validates the record on write', async () => {
     await expect(
       aliceAgent.api.com.atproto.repo.createRecord({

packages/syntax/jest.config.js

@@ -2,5 +2,5 @@ const base = require('../../jest.config.base.js')
 
 module.exports = {
   ...base,
-  displayName: 'Identifier',
+  displayName: 'Syntax',
 }

packages/syntax/src/index.ts

@@ -2,4 +2,6 @@ export * from './handle'
 export * from './did'
 export * from './nsid'
 export * from './aturi'
+export * from './tid'
+export * from './recordkey'
 export * from './datetime'

packages/syntax/src/recordkey.ts

@@ -0,0 +1,26 @@
+export const ensureValidRecordKey = (rkey: string): void => {
+  if (rkey.length > 512 || rkey.length < 1) {
+    throw new InvalidRecordKeyError('record key must be 1 to 512 characters')
+  }
+  // simple regex to enforce most constraints via just regex and length.
+  if (!/^[a-zA-Z0-9_~.-]{1,512}$/.test(rkey)) {
+    throw new InvalidRecordKeyError('record key syntax not valid (regex)')
+  }
+  if (rkey == '.' || rkey == '..')
+    throw new InvalidRecordKeyError('record key can not be "." or ".."')
+}
+
+export const isValidRecordKey = (rkey: string): boolean => {
+  try {
+    ensureValidRecordKey(rkey)
+  } catch (err) {
+    if (err instanceof InvalidRecordKeyError) {
+      return false
+    }
+    throw err
+  }
+
+  return true
+}
+
+export class InvalidRecordKeyError extends Error {}

packages/syntax/src/tid.ts

@@ -0,0 +1,24 @@
+export const ensureValidTid = (tid: string): void => {
+  if (tid.length != 13) {
+    throw new InvalidTidError('TID must be 13 characters')
+  }
+  // simple regex to enforce most constraints via just regex and length.
+  if (!/^[234567abcdefghij][234567abcdefghijklmnopqrstuvwxyz]{12}$/.test(tid)) {
+    throw new InvalidTidError('TID syntax not valid (regex)')
+  }
+}
+
+export const isValidTid = (tid: string): boolean => {
+  try {
+    ensureValidTid(tid)
+  } catch (err) {
+    if (err instanceof InvalidTidError) {
+      return false
+    }
+    throw err
+  }
+
+  return true
+}
+
+export class InvalidTidError extends Error {}

packages/syntax/tests/interop-files/recordkey_syntax_invalid.txt

@@ -0,0 +1 @@
+../../../../interop-test-files/syntax/recordkey_syntax_invalid.txt

packages/syntax/tests/interop-files/recordkey_syntax_valid.txt

@@ -0,0 +1 @@
+../../../../interop-test-files/syntax/recordkey_syntax_valid.txt

packages/syntax/tests/interop-files/tid_syntax_invalid.txt

@@ -0,0 +1 @@
+../../../../interop-test-files/syntax/tid_syntax_invalid.txt

packages/syntax/tests/interop-files/tid_syntax_valid.txt

@@ -0,0 +1 @@
+../../../../interop-test-files/syntax/tid_syntax_valid.txt

packages/syntax/tests/recordkey.test.ts

@@ -0,0 +1,42 @@
+import { ensureValidRecordKey, InvalidRecordKeyError } from '../src'
+import * as readline from 'readline'
+import * as fs from 'fs'
+
+describe('recordkey validation', () => {
+  const expectValid = (r: string) => {
+    ensureValidRecordKey(r)
+  }
+  const expectInvalid = (r: string) => {
+    expect(() => ensureValidRecordKey(r)).toThrow(InvalidRecordKeyError)
+  }
+
+  it('conforms to interop valid recordkey', () => {
+    const lineReader = readline.createInterface({
+      input: fs.createReadStream(
+        `${__dirname}/interop-files/recordkey_syntax_valid.txt`,
+      ),
+      terminal: false,
+    })
+    lineReader.on('line', (line) => {
+      if (line.startsWith('#') || line.length == 0) {
+        return
+      }
+      expectValid(line)
+    })
+  })
+
+  it('conforms to interop invalid recordkeys', () => {
+    const lineReader = readline.createInterface({
+      input: fs.createReadStream(
+        `${__dirname}/interop-files/recordkey_syntax_invalid.txt`,
+      ),
+      terminal: false,
+    })
+    lineReader.on('line', (line) => {
+      if (line.startsWith('#') || line.length == 0) {
+        return
+      }
+      expectInvalid(line)
+    })
+  })
+})

packages/syntax/tests/tid.test.ts

@@ -0,0 +1,42 @@
+import { ensureValidTid, InvalidTidError } from '../src'
+import * as readline from 'readline'
+import * as fs from 'fs'
+
+describe('tid validation', () => {
+  const expectValid = (t: string) => {
+    ensureValidTid(t)
+  }
+  const expectInvalid = (t: string) => {
+    expect(() => ensureValidTid(t)).toThrow(InvalidTidError)
+  }
+
+  it('conforms to interop valid tid', () => {
+    const lineReader = readline.createInterface({
+      input: fs.createReadStream(
+        `${__dirname}/interop-files/tid_syntax_valid.txt`,
+      ),
+      terminal: false,
+    })
+    lineReader.on('line', (line) => {
+      if (line.startsWith('#') || line.length == 0) {
+        return
+      }
+      expectValid(line)
+    })
+  })
+
+  it('conforms to interop invalid tids', () => {
+    const lineReader = readline.createInterface({
+      input: fs.createReadStream(
+        `${__dirname}/interop-files/tid_syntax_invalid.txt`,
+      ),
+      terminal: false,
+    })
+    lineReader.on('line', (line) => {
+      if (line.startsWith('#') || line.length == 0) {
+        return
+      }
+      expectInvalid(line)
+    })
+  })
+})