Fix: Hardening followups

chain-api/src/types/ChainObject.spec.ts

@@ -100,10 +100,40 @@ it("rejects a NUL character inside a composite key part", () => {
   expect(() => ChainObject.getCompositeKeyFromParts("outer", [nested])).not.toThrow();
 });
 
-it("rejects the string key separator inside a part", () => {
+it("rejects composite key parts that only look like nested composite keys", () => {
+  // Given
+  const nul = "\u0000";
+  const first = [`${nul}a${nul}`, `${nul}b${nul}${nul}${nul}c${nul}`];
+  const second = [`${nul}a${nul}${nul}${nul}b${nul}`, `${nul}c${nul}`];
+
+  // When & Then
+  expect(() => ChainObject.getCompositeKeyFromParts("idx", first)).toThrow(/NUL/);
+  expect(() => ChainObject.getCompositeKeyFromParts("idx", second)).toThrow(/NUL/);
+  expect(() => ChainObject.getCompositeKeyFromParts("idx", [nul])).toThrow(/NUL/);
+  expect(() => ChainObject.getCompositeKeyFromParts("idx", [`${nul}${nul}`])).toThrow(/NUL/);
+  expect(() => ChainObject.joinKeyParts(first)).toThrow(/NUL/);
+});
+
+it("accepts a well-formed nested composite key as a part", () => {
+  // Given
+  const nested = ChainObject.getCompositeKeyFromParts("inner", ["1700000000000", "tx1"]);
+
+  // When
+  const key = ChainObject.getCompositeKeyFromParts("outer", [nested, "client|user"]);
+
+  // Then
+  expect(key).toEqual(`\u0000outer\u0000${nested}\u0000client|user\u0000`);
+  expect(ChainObject.joinKeyParts([nested, "x"])).toEqual(`${nested}\u0000x`);
+});
+
+it("allows $ only as the first character of a string key part", () => {
+  expect(ChainObject.getStringKeyFromParts(["aaa", "$bb"])).toEqual("aaa$$bb");
+  expect(ChainObject.getPartsFromStringKey("aaa$$bb")).toEqual(["aaa", "$bb"]);
   expect(ChainObject.getStringKeyFromParts(["a", "b_c"])).toEqual("a$b_c");
-  expect(() => ChainObject.getStringKeyFromParts(["a$b", "c"])).toThrow(/separator/);
-  expect(() => ChainObject.getStringKeyFromParts(["a\u0000b"])).toThrow(/separator/);
+  expect(ChainObject.getPartsFromStringKey("a$b_c")).toEqual(["a", "b_c"]);
+  expect(() => ChainObject.getStringKeyFromParts(["aaa$", "bb"])).toThrow(/first character/);
+  expect(() => ChainObject.getStringKeyFromParts(["aa$a", "bb"])).toThrow(/first character/);
+  expect(() => ChainObject.getStringKeyFromParts(["a\u0000b"])).toThrow(/NUL/);
 });
 
 it("rejects a NUL character in a joined query prefix", () => {

chain-api/src/types/ChainObject.ts

@@ -150,25 +150,55 @@ export abstract class ChainObject {
     }
 
     // A complete composite key is sometimes stored as one component of another key.
-    // It starts and ends with the separator. Any other embedded separator is ambiguous.
-    const isNestedCompositeKey = text.startsWith(nul) && text.endsWith(nul) && text.length > 1;
-    if (!isNestedCompositeKey) {
+    // It must be well formed: the separator, then non-empty segments each followed by the separator.
+    // Any other embedded separator is ambiguous.
+    if (!ChainObject.isNestedCompositeKey(text)) {
       throw new InvalidCompositeKeyError(`${label} contains a NUL character`);
     }
     return text;
   }
 
+  private static isNestedCompositeKey(text: string): boolean {
+    const nul = ChainObject.MIN_UNICODE_RUNE_VALUE;
+    if (text.length < 3 || !text.startsWith(nul) || !text.endsWith(nul)) {
+      return false;
+    }
+    const segments = text.slice(1, -1).split(nul);
+    return segments.every((segment) => segment.length > 0);
+  }
+
   public static getStringKeyFromParts(parts: string[]): string {
+    // "$" may be the first character of a part. "aaa" + "$bb" is "aaa$$bb".
+    // A "$" later in the part is rejected, so "aaa$" + "bb" cannot produce the same string.
     const texts = parts.map((part, index) => {
       const text = part === null || part === undefined ? "" : String(part);
-      if (text.includes(ChainObject.ID_SPLIT_CHAR) || text.includes(ChainObject.MIN_UNICODE_RUNE_VALUE)) {
-        throw new InvalidCompositeKeyError(`string key part at index ${index} contains the separator`);
+      if (text.includes(ChainObject.MIN_UNICODE_RUNE_VALUE)) {
+        throw new InvalidCompositeKeyError(`string key part at index ${index} contains a NUL character`);
+      }
+      if (text.indexOf(ChainObject.ID_SPLIT_CHAR) > 0) {
+        throw new InvalidCompositeKeyError(
+          `string key part at index ${index} contains $ after its first character`
+        );
       }
       return text;
     });
     return texts.join(ChainObject.ID_SPLIT_CHAR);
   }
 
+  public static getPartsFromStringKey(stringKey: string): string[] {
+    const raw = stringKey.split(ChainObject.ID_SPLIT_CHAR);
+    const parts: string[] = [];
+    for (let i = 0; i < raw.length; i++) {
+      if (raw[i] === "" && i + 1 < raw.length) {
+        parts.push(`${ChainObject.ID_SPLIT_CHAR}${raw[i + 1]}`);
+        i += 1;
+      } else {
+        parts.push(raw[i]);
+      }
+    }
+    return parts;
+  }
+
   public static getEncodableStringKeyFromParts(parts: string[]): string {
     parts.forEach((part, index) => {
       if (part.includes(ChainObject.MIN_UNICODE_RUNE_VALUE)) {

chain-api/src/types/OracleDefinition.ts

@@ -15,6 +15,7 @@
 import { ArrayNotEmpty, IsNotEmpty, IsString } from "class-validator";
 
 import { ChainKey } from "../utils";
+import { IsUserAlias } from "../validators";
 import { ChainObject } from "./ChainObject";
 
 export class OracleDefinition extends ChainObject {
@@ -26,7 +27,6 @@ export class OracleDefinition extends ChainObject {
   public name: string;
 
   @ArrayNotEmpty()
-  @IsString({ each: true })
-  @IsNotEmpty({ each: true })
+  @IsUserAlias({ each: true })
   public authorities: string[];
 }

chain-api/src/types/RangedChainObject.spec.ts

@@ -55,7 +55,11 @@ it("rejects a NUL character inside a ranged key part", () => {
   expect(() => RangedChainObject.getRangedKeyFromParts("idx", ["a\u0000b", "c"])).toThrow(/NUL/);
   expect(() => RangedChainObject.getRangedKeyFromParts("id\u0000x", ["a"])).toThrow(/NUL/);
   expect(RangedChainObject.getStringKeyFromParts(["a", "b_c"])).toEqual("a$b_c");
-  expect(() => RangedChainObject.getStringKeyFromParts(["a$b", "c"])).toThrow(/separator/);
+  expect(RangedChainObject.getStringKeyFromParts(["aaa", "$bb"])).toEqual("aaa$$bb");
+  expect(() => RangedChainObject.getStringKeyFromParts(["a$b", "c"])).toThrow(/first character/);
+  expect(() => RangedChainObject.getRangedKeyFromParts("idx", ["\u0000a\u0000\u0000\u0000b\u0000"])).toThrow(
+    /NUL/
+  );
 });
 
 it("should use custom serializers while constructing ranged key", () => {

chain-api/src/types/TokenClass.spec.ts

@@ -176,3 +176,29 @@ it("should encode and decode successfully with utf 0 in keys", async () => {
   expect(encode).toThrow("Invalid part with UTF-0 at index 2 passed");
   expect(decode).toThrow("Expected 4 parts, got 5 parts");
 });
+
+it("should build a string key when a key field starts with $", () => {
+  // Given
+  const classKey = new TokenClassKey();
+  classKey.collection = "aaa";
+  classKey.category = "$bb";
+  classKey.type = "Key";
+  classKey.additionalKey = "none";
+
+  // When
+  const stringKey = classKey.toStringKey();
+
+  // Then
+  expect(stringKey).toEqual("aaa$$bb$Key$none");
+  expect(ChainObject.getPartsFromStringKey(stringKey)).toEqual(["aaa", "$bb", "Key", "none"]);
+});
+
+it("rejects $ after the first character of a token class key field", () => {
+  const classKey = new TokenClassKey();
+  classKey.collection = "aaa";
+  classKey.category = "Cl$ass";
+  classKey.type = "Key";
+  classKey.additionalKey = "none";
+
+  expect(() => classKey.toStringKey()).toThrow(/first character/);
+});

chain-api/src/types/TokenInstance.spec.ts

@@ -95,3 +95,20 @@ it("builds an instance key from the whole integer", () => {
     ChainObject.getCompositeKeyFromParts(TokenInstance.INDEX_KEY, ["collection", "12"])
   );
 });
+
+it("should build a string key when a key field starts with $", () => {
+  // Given
+  const instance = new TokenInstanceKey();
+  instance.collection = "aaa";
+  instance.category = "$bb";
+  instance.type = "Large";
+  instance.additionalKey = "none";
+  instance.instance = new BigNumber(0);
+
+  // When
+  const stringKey = instance.toStringKey();
+
+  // Then
+  expect(stringKey).toEqual("aaa$$bb$Large$none$0");
+  expect(ChainObject.getPartsFromStringKey(stringKey)).toEqual(["aaa", "$bb", "Large", "none", "0"]);
+});

chain-api/src/types/TokenInstance.ts

@@ -356,7 +356,7 @@ export class TokenInstance extends ChainObject {
   // This parses a string into the key that can be used to query HLF
   // This looks to be built to only handle the key to be composed of two parts and seperated by a |
   public static GetCompositeKeyFromString(tokenCid: string): string {
-    const idParts = tokenCid.split(ChainObject.ID_SPLIT_CHAR);
+    const idParts = ChainObject.getPartsFromStringKey(tokenCid);
 
     const instanceText = idParts[1];
     const canonicalInstance = /^(0|[1-9]\d*)$/.test(instanceText ?? "");

chain-api/src/types/burn.spec.ts

@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) Gala Games Inc. All rights reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import { plainToInstance } from "class-transformer";
+import { validate } from "class-validator";
+
+import { FetchBurnsDto } from "./burn";
+
+describe("FetchBurnsDto", () => {
+  it("rejects created when a preceding key field is missing", async () => {
+    // Given
+    const dto = plainToInstance(FetchBurnsDto, {
+      burnedBy: "client|user1",
+      collection: "Test",
+      created: 1700000000000
+    });
+
+    // When
+    const errors = await validate(dto);
+
+    // Then
+    expect(errors.map((e) => e.property)).toEqual(
+      expect.arrayContaining(["category", "type", "additionalKey", "instance"])
+    );
+  });
+
+  it("accepts created when all preceding key fields are present", async () => {
+    // Given
+    const dto = plainToInstance(FetchBurnsDto, {
+      burnedBy: "client|user1",
+      collection: "Test",
+      category: "Item",
+      type: "Sword",
+      additionalKey: "none",
+      instance: "1",
+      created: 1700000000000
+    });
+
+    // When
+    const errors = await validate(dto);
+
+    // Then
+    expect(errors).toHaveLength(0);
+  });
+});

chain-api/src/types/burn.ts

@@ -56,7 +56,8 @@ export class FetchBurnsDto extends ChainCallDTO {
       burn.category !== undefined ||
       burn.type !== undefined ||
       burn.additionalKey !== undefined ||
-      burn.instance !== undefined
+      burn.instance !== undefined ||
+      burn.created !== undefined
   )
   @IsString()
   @IsNotEmpty()
@@ -70,7 +71,8 @@ export class FetchBurnsDto extends ChainCallDTO {
       burn.category !== undefined ||
       burn.type !== undefined ||
       burn.additionalKey !== undefined ||
-      burn.instance !== undefined
+      burn.instance !== undefined ||
+      burn.created !== undefined
   )
   @IsString()
   @IsNotEmpty()
@@ -81,7 +83,10 @@ export class FetchBurnsDto extends ChainCallDTO {
   })
   @ValidateIf(
     (burn: FetchBurnsDto) =>
-      burn.type !== undefined || burn.additionalKey !== undefined || burn.instance !== undefined
+      burn.type !== undefined ||
+      burn.additionalKey !== undefined ||
+      burn.instance !== undefined ||
+      burn.created !== undefined
   )
   @IsString()
   @IsNotEmpty()
@@ -90,15 +95,18 @@ export class FetchBurnsDto extends ChainCallDTO {
   @JSONSchema({
     description: "Token additionalKey. Optional, but required if instance is provided."
   })
-  @ValidateIf((burn: FetchBurnsDto) => burn.additionalKey !== undefined || burn.instance !== undefined)
+  @ValidateIf(
+    (burn: FetchBurnsDto) =>
+      burn.additionalKey !== undefined || burn.instance !== undefined || burn.created !== undefined
+  )
   @IsString()
   @IsNotEmpty()
   additionalKey?: string;
 
   @JSONSchema({
-    description: "Token instance. Optional."
+    description: "Token instance. Optional, but required if created is provided."
   })
-  @ValidateIf((burn: FetchBurnsDto) => burn.instance !== undefined)
+  @ValidateIf((burn: FetchBurnsDto) => burn.instance !== undefined || burn.created !== undefined)
   @IsString()
   @IsNotEmpty()
   instance?: string;

chain-api/src/types/oracle.spec.ts

@@ -16,6 +16,7 @@ import BigNumber from "bignumber.js";
 import { plainToInstance } from "class-transformer";
 import { validate } from "class-validator";
 
+import { OracleDefinition } from "./OracleDefinition";
 import { ExternalToken } from "./OraclePriceAssertion";
 import { TokenInstanceKey } from "./TokenInstance";
 import { createValidDTO, createValidSubmitDTO } from "./dtos";
@@ -234,4 +235,31 @@ describe("oracle.ts", () => {
       })
     ).rejects.toThrow(/authorities/);
   });
+
+  test.each([["   "], ["foo"], ["0x0000000000000000000000000000000000000001"]])(
+    "oracle authorities reject %p as it is not a user alias",
+    async (authority) => {
+      // When & Then
+      await expect(
+        createValidSubmitDTO(OracleDefinitionDto, {
+          name: mockOracle,
+          authorities: ["client|oracle", authority]
+        })
+      ).rejects.toThrow(/authorities/);
+    }
+  );
+
+  test("oracle definition rejects authorities that are not user aliases", async () => {
+    // Given
+    const definition = plainToInstance(OracleDefinition, {
+      name: mockOracle,
+      authorities: ["client|oracle", "EthereumBridge", " "]
+    });
+
+    // When
+    const errors = await validate(definition);
+
+    // Then
+    expect(errors.map((e) => e.property)).toEqual(["authorities"]);
+  });
 });

chain-api/src/types/oracle.ts

@@ -35,7 +35,7 @@ import {
 import { JSONSchema } from "class-validator-jsonschema";
 
 import { ValidationFailedError } from "../utils";
-import { BigNumberProperty } from "../validators";
+import { BigNumberProperty, IsUserAlias } from "../validators";
 import { OracleDefinition } from "./OracleDefinition";
 import {
   ExactlyOneBaseTokenConstraint,
@@ -67,8 +67,7 @@ export class OracleDefinitionDto extends SubmitCallDTO {
   @IsOptional()
   @ArrayNotEmpty()
   @ArrayUnique()
-  @IsString({ each: true })
-  @IsNotEmpty({ each: true })
+  @IsUserAlias({ each: true })
   public authorities?: string[];
 }
 

chain-api/src/types/token.ts

@@ -411,6 +411,22 @@ export class FetchBalancesDto extends ChainCallDTO {
   })
   @IsOptional()
   additionalKey?: string;
+
+  @JSONSchema({
+    description: "Page bookmark. If it is undefined, then the first page is returned."
+  })
+  @IsOptional()
+  @IsNotEmpty()
+  bookmark?: string;
+
+  @JSONSchema({
+    description: "Page size. When omitted, the query uses the total results limit. Maximum 10000."
+  })
+  @IsOptional()
+  @Max(10 * 1000)
+  @Min(1)
+  @IsInt()
+  limit?: number;
 }
 
 @JSONSchema({