double check and fix review

Author: Zetazzz

graphile/graphile-misc-plugins/__tests__/PublicKeySignature.test.ts

@@ -65,6 +65,12 @@ describe('PublicKeySignature config validation', () => {
     );
   });
 
+  it('throws on invalid crypto_network value', () => {
+    expect(() => PublicKeySignature({ ...defaultConfig, crypto_network: 'btc mainnet' })).toThrow(
+      /invalid crypto_network/,
+    );
+  });
+
   it('throws on function name with uppercase letters', () => {
     expect(() => PublicKeySignature({ ...defaultConfig, sign_in_request_challenge: 'BadName' })).toThrow(
       /invalid sign_in_request_challenge/,

graphile/graphile-misc-plugins/src/PublicKeySignature.ts

@@ -16,21 +16,31 @@ export interface PublicKeyChallengeConfig {
 }
 
 const SAFE_IDENTIFIER = /^[a-z_][a-z0-9_]*$/;
+const SAFE_CRYPTO_NETWORK = /^[a-z0-9_-]{1,64}$/i;
 
 function validateIdentifier(name: string, label: string): void {
   if (!SAFE_IDENTIFIER.test(name)) {
     throw new Error(`PublicKeySignature: invalid ${label} "${name}" — must match /^[a-z_][a-z0-9_]*$/`);
   }
 }
 
+function validateCryptoNetwork(name: string): void {
+  if (!SAFE_CRYPTO_NETWORK.test(name)) {
+    throw new Error(
+      'PublicKeySignature: invalid crypto_network — must match /^[a-z0-9_-]{1,64}$/i',
+    );
+  }
+}
+
 const MAX_PUBLIC_KEY_LENGTH = 256;
 const MAX_MESSAGE_LENGTH = 4096;
 const MAX_SIGNATURE_LENGTH = 1024;
+const ENABLE_SIGNATURE_VERIFICATION = process.env.ENABLE_SIGNATURE_VERIFICATION === 'true';
 
 export const PublicKeySignature = (pubkey_challenge: PublicKeyChallengeConfig): GraphileConfig.Plugin => {
   const {
     schema,
-    crypto_network: _crypto_network,
+    crypto_network,
     sign_up_with_key,
     sign_in_request_challenge,
     sign_in_record_failure,
@@ -42,6 +52,7 @@ export const PublicKeySignature = (pubkey_challenge: PublicKeyChallengeConfig):
   validateIdentifier(sign_in_request_challenge, 'sign_in_request_challenge');
   validateIdentifier(sign_in_record_failure, 'sign_in_record_failure');
   validateIdentifier(sign_in_with_challenge, 'sign_in_with_challenge');
+  validateCryptoNetwork(crypto_network);
 
   return extendSchema(() => ({
     typeDefs: gql`
@@ -157,11 +168,8 @@ export const PublicKeySignature = (pubkey_challenge: PublicKeyChallengeConfig):
           });
         },
 
-        // NOTE: Crypto verification is currently stubbed (always returns false).
-        // The verifyMessage call from @pyramation/crypto-keys needs to be
-        // re-implemented, e.g. using interchainJS, before this mutation can
-        // succeed. Until then, verifyMessageForSigning will always record a
-        // failure and throw BAD_SIGNIN.
+        // NOTE: Verification remains behind a feature flag until crypto
+        // verification is re-implemented.
         verifyMessageForSigning(_$mutation: any, fieldArgs: any) {
           const $input = fieldArgs.getRaw('input');
           const $withPgClient = (grafastContext() as any).get('withPgClient');
@@ -180,24 +188,13 @@ export const PublicKeySignature = (pubkey_challenge: PublicKeyChallengeConfig):
               throw new Error('INVALID_SIGNATURE');
             }
 
-            // TODO: Re-implement crypto verification (e.g. using interchainJS).
-            // const network = Networks[crypto_network];
-            // const result = verifyMessage(message, publicKey, signature, network);
-            const result = false;
+            if (!ENABLE_SIGNATURE_VERIFICATION) {
+              // Fail closed without mutating lockout counters while verification
+              // is disabled.
+              throw new Error('FEATURE_DISABLED');
+            }
 
             return withPgClient(null, async (pgClient: any) => {
-              if (!result) {
-                // Record failure outside transaction — must persist for rate limiting/lockout
-                await pgQueryWithContext({
-                  client: pgClient,
-                  context: { role: 'anonymous' },
-                  query: `SELECT * FROM "${schema}"."${sign_in_record_failure}"($1)`,
-                  variables: [publicKey],
-                  skipTransaction: true
-                });
-                throw new Error('BAD_SIGNIN');
-              }
-
               // Only the success path needs a transaction (multi-step)
               await pgClient.query('BEGIN');
               try {

graphile/graphile-plugin-connection-filter-postgis/src/plugin.ts

@@ -5,6 +5,22 @@ import type { SQL } from 'pg-sql2';
 import { CONCRETE_SUBTYPES } from 'graphile-postgis';
 import type { PostgisExtensionInfo } from 'graphile-postgis';
 
+const ALLOWED_SQL_OPERATORS = new Set([
+  '=',
+  '&&',
+  '&&&',
+  '&<',
+  '&<|',
+  '&>',
+  '|&>',
+  '<<',
+  '<<|',
+  '>>',
+  '|>>',
+  '~',
+  '~=',
+]);
+
 /**
  * PgConnectionArgFilterPostgisOperatorsPlugin
  *
@@ -257,6 +273,10 @@ export const PgConnectionArgFilterPostgisOperatorsPlugin: GraphileConfig.Plugin
 
         // Process SQL operator-based operators
         for (const [op, baseTypes, operatorName, description] of operatorSpecs) {
+          if (!ALLOWED_SQL_OPERATORS.has(op)) {
+            throw new Error(`Unexpected SQL operator: ${op}`);
+          }
+
           for (const baseType of baseTypes) {
             allSpecs.push({
               typeNames: gqlTypeNamesByBase[baseType],

graphile/graphile-settings/__tests__/upload-resolver.test.ts

@@ -95,7 +95,7 @@ describe('uploadResolver MIME validation', () => {
         {},
         { uploadPlugin: { tags: {}, type: 'image' } },
       ),
-    ).rejects.toThrow('UPLOAD_MIMETYPE image/jpg,image/jpeg,image/png,image/svg+xml');
+    ).rejects.toThrow('UPLOAD_MIMETYPE image/jpeg,image/png,image/svg+xml');
 
     expect(mockDetectContentType).toHaveBeenCalledTimes(1);
     expect(mockUploadWithContentType).not.toHaveBeenCalled();
@@ -141,4 +141,3 @@ describe('uploadResolver MIME validation', () => {
     );
   });
 });
-

graphile/graphile-settings/src/upload-resolver.ts

@@ -20,6 +20,7 @@ import Streamer from '@constructive-io/s3-streamer';
 import uploadNames from '@constructive-io/upload-names';
 import { getEnvOptions } from '@constructive-io/graphql-env';
 import { Logger } from '@pgpmjs/logger';
+import { randomBytes } from 'crypto';
 import type { Readable } from 'stream';
 import type {
 	FileUpload,
@@ -28,6 +29,7 @@ import type {
 } from 'graphile-upload-plugin';
 
 const log = new Logger('upload-resolver');
+const DEFAULT_IMAGE_MIME_TYPES = ['image/jpeg', 'image/png', 'image/svg+xml'];
 
 let streamer: Streamer | null = null;
 let bucketName: string;
@@ -66,9 +68,7 @@ function getStreamer(): Streamer {
  * Format: {random10chars}-{sanitized-filename}
  */
 function generateKey(filename: string): string {
-	const rand =
-		Math.random().toString(36).substring(2, 7) +
-		Math.random().toString(36).substring(2, 7);
+	const rand = randomBytes(12).toString('hex');
 	return `${rand}-${uploadNames(filename)}`;
 }
 
@@ -124,9 +124,9 @@ async function uploadResolver(
 				.trim()
 				.split(',')
 				.map((a: string) => a.trim())
-			: typ === 'image'
-				? ['image/jpg', 'image/jpeg', 'image/png', 'image/svg+xml']
-				: [];
+		: typ === 'image'
+			? DEFAULT_IMAGE_MIME_TYPES
+			: [];
 
 	const detected = await s3.detectContentType({
 		readStream: upload.createReadStream(),

graphile/graphile-sql-expression-validator/__tests__/plugin.test.ts

@@ -280,6 +280,40 @@ describe('GraphQLObjectType_fields_field hook behavior', () => {
     ).rejects.toThrow('Invalid SQL expression in formula');
   });
 
+  it('should validate deeply nested text input in sideEffect callback', async () => {
+    const plugin = createSqlExpressionValidatorPlugin();
+    const hook = plugin.schema!.hooks!
+      .GraphQLObjectType_fields_field as Function;
+
+    const field = {
+      type: 'CreateWidgetPayload',
+      plan: jest.fn().mockReturnValue('step')
+    };
+    const build = makeMockBuild();
+    const context = makeMockContext({
+      isRootMutation: true,
+      pgCodec: {
+        attributes: {
+          formula: {
+            extensions: { tags: { sqlExpression: true } }
+          }
+        }
+      }
+    });
+
+    const result = hook(field, build, context);
+    const mockFieldArgs = {
+      get: jest.fn().mockReturnValue('inputStep')
+    };
+    result.plan('parent', mockFieldArgs);
+
+    const validationCallback = mockSideEffect.mock.calls[0][1];
+
+    await expect(
+      validationCallback({ level1: { level2: { formula: 'username' } } })
+    ).rejects.toThrow('Invalid SQL expression in formula');
+  });
+
   it('should skip null/undefined values in sideEffect callback', async () => {
     const plugin = createSqlExpressionValidatorPlugin();
     const hook = plugin.schema!.hooks!

graphile/graphile-sql-expression-validator/__tests__/validator.test.ts

@@ -96,6 +96,18 @@ describe('parseAndValidateSqlExpression', () => {
       expect(result.valid).toBe(false);
       expect(result.error).toContain('semicolons');
     });
+
+    it('should reject line comments', async () => {
+      const result = await parseAndValidateSqlExpression('now() -- trailing');
+      expect(result.valid).toBe(false);
+      expect(result.error).toContain('SQL comments');
+    });
+
+    it('should reject block comments', async () => {
+      const result = await parseAndValidateSqlExpression('now() /* comment */');
+      expect(result.valid).toBe(false);
+      expect(result.error).toContain('SQL comments');
+    });
   });
 
   // ─── Valid expressions ───────────────────────────────────────────
@@ -367,6 +379,14 @@ describe('parseAndValidateSqlExpression', () => {
       expect(resultA.valid).toBe(true);
       expect(resultB.valid).toBe(true);
     });
+
+    it('should match allowed schema names case-insensitively', async () => {
+      const result = await parseAndValidateSqlExpression(
+        'app_public.my_func()',
+        { allowedSchemas: ['APP_PUBLIC'], allowedFunctions: ['my_func'] }
+      );
+      expect(result.valid).toBe(true);
+    });
   });
 
   // ─── Allowlist rejection (disallowed node types) ────────────────

graphile/graphile-sql-expression-validator/src/plugin.ts

@@ -37,16 +37,39 @@ import type { SqlExpressionValidatorOptions } from './validator';
 
 /**
  * Find a value in a potentially nested mutation input object.
- * PostGraphile v5 mutation input is structured as `{ tablePatch: { field: value } }`
- * or `{ table: { field: value } }` — at most one level of nesting.
+ * Input is typically 1 level deep, but we traverse defensively to avoid
+ * silently skipping deeper/nested mutation payloads.
  */
 function findValue(input: Record<string, unknown>, key: string): unknown {
-  if (key in input) return input[key];
-  for (const v of Object.values(input)) {
-    if (v && typeof v === 'object' && key in (v as Record<string, unknown>)) {
-      return (v as Record<string, unknown>)[key];
+  const visited = new Set<object>();
+  const stack: Array<{ node: Record<string, unknown>; depth: number }> = [
+    { node: input, depth: 0 }
+  ];
+  const maxDepth = 32;
+
+  while (stack.length > 0) {
+    const current = stack.pop()!;
+    const { node, depth } = current;
+
+    if (key in node) {
+      return node[key];
+    }
+
+    if (visited.has(node) || depth >= maxDepth) {
+      continue;
+    }
+    visited.add(node);
+
+    for (const value of Object.values(node)) {
+      if (value && typeof value === 'object') {
+        stack.push({
+          node: value as Record<string, unknown>,
+          depth: depth + 1
+        });
+      }
     }
   }
+
   return undefined;
 }
 

graphile/graphile-sql-expression-validator/src/validator.ts

@@ -149,6 +149,7 @@ const FORBIDDEN_TYPE_CASTS = new Set([
 ]);
 
 const MAX_AST_DEPTH = 100;
+const SQL_COMMENT_TOKENS = /--|\/\*|\*\//;
 
 // ─── Internal helpers ─────────────────────────────────────────────
 
@@ -179,7 +180,7 @@ function extractStringVal(n: unknown): string {
 function validateAstNode(
   node: unknown,
   allowedFunctions: ReadonlySet<string>,
-  allowedSchemas: readonly string[],
+  allowedSchemas: ReadonlySet<string>,
   path: string[]
 ): AstNodeValidationResult {
   if (path.length > MAX_AST_DEPTH) {
@@ -247,7 +248,7 @@ function validateAstNode(
       const functionName = names[names.length - 1];
 
       if (schemaName) {
-        if (!allowedSchemas.includes(schemaName)) {
+        if (!allowedSchemas.has(schemaName.toLowerCase())) {
           return {
             valid: false,
             error: `Function schema "${schemaName}" is not in the allowed schemas list`
@@ -345,7 +346,15 @@ export async function parseAndValidateSqlExpression(
     };
   }
 
+  if (SQL_COMMENT_TOKENS.test(expression)) {
+    return {
+      valid: false,
+      error: 'Expression cannot contain SQL comments'
+    };
+  }
+
   const funcSet = new Set(allowedFunctions.map((f) => f.toLowerCase()));
+  const schemaSet = new Set(allowedSchemas.map((s) => s.toLowerCase()));
 
   try {
     const wrappedSql = `SELECT (${expression})`;
@@ -381,7 +390,7 @@ export async function parseAndValidateSqlExpression(
     const validationResult = validateAstNode(
       expressionAst,
       funcSet,
-      allowedSchemas,
+      schemaSet,
       []
     );
 
@@ -434,11 +443,12 @@ export async function validateAst(
   }
 
   const funcSet = new Set(allowedFunctions.map((f) => f.toLowerCase()));
+  const schemaSet = new Set(allowedSchemas.map((s) => s.toLowerCase()));
 
   const validationResult = validateAstNode(
     ast,
     funcSet,
-    allowedSchemas,
+    schemaSet,
     []
   );