👌

TypescriptでPartial型オブジェクトを引数に使う時は気をつけよう!

に公開

本日は弊社でprismaをラップして使用している時に起きたインシデントについて皆様にお話しします。
自社への教訓も兼ねまして、皆様への注意喚起になればと思い記事にしました。

危険なケーススタディ

Partial型オブジェクトの引数は制約を一つでも満たすとWidened Typeを受け入れる

弊社サービスでは以下のように、ユーザーに運営側からデータを付与できる関数が存在していました。

async function presentItems(args: {
  accountId: string
  where?: Prisma.ItemWhereInput
}) {
    const { accountId, where } = args
    const presentItems = await prisma.items.findMany({ where })
    return prisma.accountItems.createMany({
      data: presentItems.map(presentItem => {
        return { item_id: presentItem.id, account_id: accountId }
      })
    })
}

この関数を管理画面のエンドポイントやバッチなどで提供しやくするために、エントリポイントで以下のように引数を単純にした関数でラップして利用することにしました。

async function operatePresentItems(args: {
  accountId: string,
  itemId: string[]
}) {
  return presentItems(args)
}

以上のコードはitemIdはwhereのkeyに指定されていないので、書いた人の意図通りには動作しないのですが、typescriptはこのコードの型エラーを検知しません。accountIdがpresentItemsの引数の制約を満たしていることで、引数のwideningが許容されてしまうからです。

危険なコードを実行する

この程度のコードであれば、型エラーがなくても何かおかしい事に気付けるかもしれません。
実例はもう少し複雑なargsの組み立てを行っていて、要するに誤ってpresentItemsの要求する引数のwhere fieldを含めずに、異なるfieldにwhereのデータを入れてしまいました。
例えば以下のようなコードの修正はありがちかと思います。

async function operatePresentItems(args: {
  accountId: string,
  itemId: string[]
}) {
  return presentItems(parseData(args))
}

function parseData(args: {
  accountId: string,
  itemId: string[]
}) {
  return  {
    accountId: args.accountId,
    where: {
      id: {
        in: args.itemId
      }
    }
  }
}

そして、このコードを運用しながら、ある日presentItemsをリファクタリングで引数の型を以下のように別のfieldの内部に含めるように変更しました。

async function presentItems(args: {
  accountId: string
  query?: { 
    where?: Prisma.ItemWhereInput
  }
}) {
    const { accountId, ...query } = args
    const presentItems = await prisma.items.findMany(query)
    return prisma.accountItems.createMany({
      data: presentItems.map(presentItem => {
        return { item_id: presentItem.id, account_id: accountId }
      })
    })
}

この後、型エラーが出ずbuildができたため、この変更が及ぼすロジックの変異に気づかず、定常的に実行しているタスクとしてoperatePresentItemsを実行してしまいました。

何が起きたのか

以上の例のコードはwhere条件をすり抜けます。これによってDB上に存在する全てのItemを指定したアカウントに紐づけてしまいました。
実際のコードではただレコードをcreateしていただけでなく、様々なファイルのアップロードなども走り、あわや大惨事!という自体に発展仕掛けました。処理の取り消し対応に追われましたが、不正に作られたデータの特定が容易だったために、無事事なきを得ました。

戒め

関数には値(リテラル)を直接入力すると安全性が高まる

typescriptは関数にリテラルを入力するケースと変数を入力するケースで型のnarrowing, wideningの扱いが変わります。

const args = {
  accountId: args.accountId,
  where: {
    id: {
      in: args.itemId
    }
  }
}
presentItems(args)

実例や上記のコードのように変数や引数を介して関数を入力すると、1つ以上のフィールド制約を満たした上で型制約を満たすと、不要なfield指定があっても型チェックをパスしてしまいます。これは、Reactなどの入力値の余計なフィールドを気にせずspread operatorを使うのに便利でもありますが、今回のようなケースでは危険です。

presentItems({
  accountId: args.accountId,
  where: {
    id: {
      in: args.itemId
    }
  }
})

と直接指定していれば、リファクタリング後の型変更を検知して型エラーになります。

また、余談になりますが、1つ以上のフィールドの型制約が満たされていない場合は全てoptionalの引数でもnarrowingが起きてエラーになるようです。以下は例です。

function something(arg: {a?: string}) {
}

// 制約を満たすのでエラーにならない
const arg = {}
something(arg)
// このケースでは変数でもwideningが適応されず無関係のフィールドがあるとエラーになる
const arg = {b: ''}
something(arg)

この仕様は一貫性のない挙動に見えることからtypescriptの意図した仕様かどうか怪しいところがあり、今後同様な挙動のままであるかは分からないのでご注意ください。

関数の戻り値に型をつけて値・リテラルを返す

今回の例では、parseDataが型推論に頼っていたことも事故の原因の一つです。
parseDataの戻り値に型をつけたうえで、かつ、値・リテラル型を指定していた場合は、未定義のフィールドを加えた時に型エラーにすることができます。

function parseData(args: {
  accountId: string,
  item_id: string
}): Parameters<typeof presentItems>[0] {
  return  {
    accountId: args.accountId,
    where :{
      id: args.item_id
    }
  }
}

上記の例ではリファクタリング時にパラメータがwhereからqueryに移行した場合でも、whereが未定義フィールドとみなされて型エラーになり安全です。

反変性を防ぐカスタム型を作る

以下のようなカスタムユーティリティを作ると引数の型を厳密にすることができます。

type Exact<T, Shape> = T extends Shape
  ? Exclude<keyof T, keyof Shape> extends never
    ? T
    : never
  : never;

function printUser<T>(user: Exact<T, { name: string; age: number }>) {
  console.log(user.name, user.age);
}

printUser({ name: "Alice", age: 30 }); // ✅ OK
printUser({ name: "Alice", age: 30, extra: true }); // ❌ エラー

ただし,この方法は関数の入力値のキャストや引数の展開が頻発するので、テストなどでカバーできる場合は積極的に採用しなくてもよいでしょう。チームとしての認知コストも上がるので、あくまでピンチハッチのようなものとして使うのが良いと思います。

(十分な)テストを書く

今回のケースではテストケースを書いて無かったわけでは有りません。
ただ、除外されるべきデータを含んでいるか検証するようなテストコードを含んでいませんでした。
特に選出対象を間違えると大事故につながるようなロジックは選出対象自体を確認するテストも必要だと感じました。

typescriptでも入力値と出力値の型が決められるからといって、細かいテストを省けると思わないほうがいいのは良い教訓でした。

この記事で同じような事故に遭う人が一人でも減らす事ができたら幸いです。

株式会社MOCHI

Discussion