Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
"workspaces": [
"src/api",
"src/ui",
"src/archival"
"src/archival",
"src/linkryEdgeFunction"
],
"packageManager": "yarn@1.22.22",
"scripts": {
"postinstall": "npm run setup",
"setup": "git config blame.ignoreRevsFile .git-blame-ignore-revs",
"build": "concurrently --names 'api,ui,archival' 'yarn workspace infra-core-api run build' 'yarn workspace infra-core-ui run build' 'yarn workspace infra-core-archival run build'",
"build": "concurrently --names 'api,ui,archival,linkryEdge' 'yarn workspace infra-core-api run build' 'yarn workspace infra-core-ui run build' 'yarn workspace infra-core-archival run build' 'yarn workspace infra-core-linkry-edge run build'",
"postbuild": "node src/api/createLambdaPackage.js && yarn lockfile-manage",
"dev": "cross-env DISABLE_AUDIT_LOG=true concurrently --names 'api,ui' 'yarn workspace infra-core-api run dev' 'yarn workspace infra-core-ui run dev'",
"lockfile-manage": "synp --with-workspace --source-file yarn.lock",
Expand Down Expand Up @@ -94,4 +95,4 @@
"pdfjs-dist": "^4.8.69",
"form-data": "^4.0.4"
}
}
}
4 changes: 2 additions & 2 deletions src/api/functions/linkry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
} from "@aws-sdk/client-dynamodb";
import { unmarshall } from "@aws-sdk/util-dynamodb";
import { LinkryGroupUUIDToGroupNameMap } from "common/config.js";
import { DelegatedLinkRecord, LinkRecord } from "common/types/linkry.js";
import { LinkRecord } from "common/types/linkry.js";
import { FastifyRequest } from "fastify";

export async function fetchLinkEntry(
Expand Down Expand Up @@ -255,7 +255,7 @@ export async function getDelegatedLinks(
...ownerRecord,
access: groupIds,
owner: ownerRecord.access.replace("OWNER#", ""),
} as DelegatedLinkRecord;
} as LinkRecord;
} catch (error) {
console.error(`Error processing delegated slug ${slug}:`, error);
return null;
Expand Down
7 changes: 7 additions & 0 deletions src/common/types/generic.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Organizations } from "@acm-uiuc/js-shared";
import * as z from "zod/v4";


Expand All @@ -23,3 +24,9 @@ export const illinoisNetId = z
example: "rjjones",
id: "IllinoisNetId",
});

export const OrgUniqueId = z.enum(Object.keys(Organizations)).meta({
description: "The unique org ID for a given ACM sub-organization. See https://github.com/acm-uiuc/js-shared/blob/main/src/orgs.ts#L15",
examples: ["A01", "C01"],
id: "OrgUniqueId"
})
Comment on lines +28 to +32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix formatting issues.

The code has several formatting violations that should be addressed:

  • Missing line breaks and indentation
  • Missing trailing comma in the metadata object
  • Missing semicolon at the end

Run prettier --write src/common/types/generic.ts to auto-fix these issues.

🧰 Tools
🪛 ESLint

[error] 29-29: Insert ⏎···

(prettier/prettier)


[error] 31-31: Insert ,

(prettier/prettier)


[error] 32-32: Insert ;

(prettier/prettier)

🤖 Prompt for AI Agents
In src/common/types/generic.ts around lines 28 to 32, the OrgUniqueId export is
improperly formatted: fix line breaks and indentation so the z.enum call and its
meta object are each on their own lines, add the missing trailing comma after
the meta object properties, and append a semicolon at the end of the statement;
then run prettier --write src/common/types/generic.ts (or apply your project's
formatter) to ensure consistent styling.

14 changes: 6 additions & 8 deletions src/common/types/linkry.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import * as z from "zod/v4";
import { OrgUniqueId } from "./generic.js";

export type ShortLinkEntry = {
slug: string;
access: string;
redir?: string;
};

export const LINKRY_MAX_SLUG_LENGTH = 1000;
export const LINKRY_MAX_SLUG_LENGTH = 100;

export const getRequest = z.object({
slug: z.string().min(1).max(LINKRY_MAX_SLUG_LENGTH).optional()
Expand All @@ -19,7 +20,10 @@ export const linkryAccessList = z.array(z.string().min(1)).meta({


export const createRequest = z.object({
slug: linkrySlug,
slug: linkrySlug.refine((url) => !url.includes('#'), {
message: "Slug must not contain a hashtag"
}),
Comment on lines +23 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Good validation addition, but fix formatting.

The hashtag validation is sensible since # is a special character in URLs (fragment identifier) and would interfere with slug matching. However, please fix the formatting issues flagged by ESLint (use double quotes and add trailing comma).

-  slug: linkrySlug.refine((url) => !url.includes('#'), {
-    message: "Slug must not contain a hashtag"
-  }),
+  slug: linkrySlug.refine((url) => !url.includes("#"), {
+    message: "Slug must not contain a hashtag",
+  }),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
slug: linkrySlug.refine((url) => !url.includes('#'), {
message: "Slug must not contain a hashtag"
}),
slug: linkrySlug.refine((url) => !url.includes("#"), {
message: "Slug must not contain a hashtag",
}),
🧰 Tools
🪛 ESLint

[error] 23-23: Replace '#' with "#"

(prettier/prettier)


[error] 24-24: Insert ,

(prettier/prettier)

🤖 Prompt for AI Agents
In src/common/types/linkry.ts around lines 23 to 25, the refine call uses single
quotes and is missing a trailing comma; change the string quotes to double
quotes and ensure the object literal has a trailing comma so the line reads with
double-quoted message and ends with a comma to satisfy ESLint formatting rules.

orgId: z.optional(OrgUniqueId),
access: linkryAccessList,
redirect: z.url().min(1).meta({ description: "Full URL to redirect to when the short URL is visited.", example: "https://google.com" })
});
Expand All @@ -33,14 +37,8 @@ export const linkRecord = z.object({
owner: z.string().min(1)
});

export const delegatedLinkRecord = linkRecord.extend({
owner: z.string().min(1)
});

export type LinkRecord = z.infer<typeof linkRecord>;

export type DelegatedLinkRecord = z.infer<typeof delegatedLinkRecord>;

export const getLinksResponse = z.object({
ownedLinks: z.array(linkRecord),
delegatedLinks: z.array(linkRecord)
Expand Down
42 changes: 42 additions & 0 deletions src/linkryEdgeFunction/build.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/* eslint-disable no-console */
import esbuild from "esbuild";

const commonParams = {
bundle: true,
format: "esm",
minify: true,
outExtension: { ".js": ".mjs" },
loader: {
".png": "file",
".pkpass": "file",
".json": "file",
}, // File loaders
target: "es2022", // Target ES2022
sourcemap: true,
platform: "node",
external: ["@aws-sdk/*"],
banner: {
js: `
import path from 'path';
import { fileURLToPath } from 'url';
import { createRequire as topLevelCreateRequire } from 'module';
const require = topLevelCreateRequire(import.meta.url);
const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
`.trim(),
}, // Banner for compatibility with CommonJS
};

esbuild
.build({
...commonParams,
entryPoints: ["linkryEdgeFunction/index.js"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Incorrect entry point path.

The entry point is set to "linkryEdgeFunction/index.js", but when this build script runs from the src/linkryEdgeFunction/ directory (via yarn workspace), it would look for src/linkryEdgeFunction/linkryEdgeFunction/index.js, which doesn't exist.

After TypeScript compilation (assuming the tsconfig is fixed to include the current directory), the compiled output should be at ../../dist/linkryEdgeFunction/index.js relative to this build script.

Apply this diff to fix the entry point:

-    entryPoints: ["linkryEdgeFunction/index.js"],
+    entryPoints: ["../../dist/linkryEdgeFunction/index.js"],
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
entryPoints: ["linkryEdgeFunction/index.js"],
entryPoints: ["../../dist/linkryEdgeFunction/index.js"],
🤖 Prompt for AI Agents
In src/linkryEdgeFunction/build.js around line 33, the entry point is
incorrectly set to "linkryEdgeFunction/index.js" which resolves to
src/linkryEdgeFunction/linkryEdgeFunction/index.js when run from that directory;
change the entryPoints value to point to the compiled output at
"../../dist/linkryEdgeFunction/index.js" so the bundler picks up the post-TS
compilation file (ensure the tsconfig/outDir produces that path).

outdir: "../../dist/linkryEdgeFunction/",
})
.then(() =>
console.log("Linkry Edge Function lambda build completed successfully!"),
)
.catch((error) => {
console.error("Linkry Edge Function lambda build failed:", error);
process.exit(1);
});
130 changes: 130 additions & 0 deletions src/linkryEdgeFunction/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import {
DynamoDBClient,
QueryCommand,
QueryCommandInput,
} from "@aws-sdk/client-dynamodb";
import type {
CloudFrontRequestEvent,
CloudFrontRequestResult,
} from "aws-lambda";

const DEFAULT_AWS_REGION = "us-east-2";
const AVAILABLE_REPLICAS = ["us-west-2"];
const DYNAMODB_TABLE = "infra-core-api-linkry";
const FALLBACK_URL = process.env.FALLBACK_URL || "https://acm.illinois.edu/404";
const DEFAULT_URL = process.env.DEFAULT_URL || "https://www.acm.illinois.edu";
const CACHE_TTL = "30"; // seconds to hold response in PoP

/**
* Determine which DynamoDB replica to use based on Lambda execution region
*/
function selectReplica(lambdaRegion: string): string {
// First check if Lambda is running in a replica region
if (AVAILABLE_REPLICAS.includes(lambdaRegion)) {
return lambdaRegion;
}

// Otherwise, find nearest replica by region prefix matching
const regionPrefix = lambdaRegion.split("-").slice(0, 2).join("-");
if (regionPrefix === "us") {
return DEFAULT_AWS_REGION;
}

for (const replica of AVAILABLE_REPLICAS) {
if (replica.startsWith(regionPrefix)) {
return replica;
}
}

return DEFAULT_AWS_REGION;
}
Comment on lines +21 to +40
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Logic error in region prefix matching.

On line 28, regionPrefix is constructed by taking the first two parts of the region name joined by a dash. For example, "us-east-1" becomes "us-east". However, line 29 checks if regionPrefix === "us", which would never be true since the prefix will always include the geographic area (e.g., "us-east", "us-west").

Apply this diff to fix the logic:

   // Otherwise, find nearest replica by region prefix matching
   const regionPrefix = lambdaRegion.split("-").slice(0, 2).join("-");
-  if (regionPrefix === "us") {
+  if (regionPrefix.startsWith("us")) {
     return DEFAULT_AWS_REGION;
   }

Alternatively, if the intent is to match US regions more broadly:

-  const regionPrefix = lambdaRegion.split("-").slice(0, 2).join("-");
-  if (regionPrefix === "us") {
+  const regionPrefix = lambdaRegion.split("-")[0];
+  if (regionPrefix === "us") {
     return DEFAULT_AWS_REGION;
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function selectReplica(lambdaRegion: string): string {
// First check if Lambda is running in a replica region
if (AVAILABLE_REPLICAS.includes(lambdaRegion)) {
return lambdaRegion;
}
// Otherwise, find nearest replica by region prefix matching
const regionPrefix = lambdaRegion.split("-").slice(0, 2).join("-");
if (regionPrefix === "us") {
return DEFAULT_AWS_REGION;
}
for (const replica of AVAILABLE_REPLICAS) {
if (replica.startsWith(regionPrefix)) {
return replica;
}
}
return DEFAULT_AWS_REGION;
}
function selectReplica(lambdaRegion: string): string {
// First check if Lambda is running in a replica region
if (AVAILABLE_REPLICAS.includes(lambdaRegion)) {
return lambdaRegion;
}
// Otherwise, find nearest replica by region prefix matching
const regionPrefix = lambdaRegion.split("-").slice(0, 2).join("-");
if (regionPrefix.startsWith("us")) {
return DEFAULT_AWS_REGION;
}
for (const replica of AVAILABLE_REPLICAS) {
if (replica.startsWith(regionPrefix)) {
return replica;
}
}
return DEFAULT_AWS_REGION;
}
🤖 Prompt for AI Agents
In src/linkryEdgeFunction/index.ts around lines 21 to 40, the code builds
regionPrefix as the first two segments (e.g., "us-east") but then checks
regionPrefix === "us" which never matches; fix by either computing regionPrefix
as only the first segment (const regionPrefix = lambdaRegion.split("-")[0];) and
keep the existing equality check, or keep the two-segment prefix and change the
conditional to test for US regions with startsWith (if
(regionPrefix.startsWith("us-")) { return DEFAULT_AWS_REGION; }), and ensure the
rest of the loop still searches AVAILABLE_REPLICAS using the chosen prefix
logic.


const currentRegion = process.env.AWS_REGION || DEFAULT_AWS_REGION;
const targetRegion = selectReplica(currentRegion);
const dynamodb = new DynamoDBClient({ region: targetRegion });

console.log(`Lambda in ${currentRegion}, routing DynamoDB to ${targetRegion}`);

export const handler = async (
event: CloudFrontRequestEvent,
): Promise<CloudFrontRequestResult> => {
const request = event.Records[0].cf.request;
const path = request.uri.replace(/^\/+/, "");

console.log(`Processing path: ${path}`);

if (!path) {
return {
status: "301",
statusDescription: "Moved Permanently",
headers: {
location: [{ key: "Location", value: DEFAULT_URL }],
"cache-control": [
{ key: "Cache-Control", value: `public, max-age=${CACHE_TTL}` },
],
},
};
}

// Query DynamoDB for records with PK=path and SK starting with "OWNER#"
try {
const queryParams: QueryCommandInput = {
TableName: DYNAMODB_TABLE,
KeyConditionExpression:
"slug = :slug AND begins_with(access, :owner_prefix)",
ExpressionAttributeValues: {
":slug": { S: path },
":owner_prefix": { S: "OWNER#" },
},
ProjectionExpression: "redirect",
Limit: 1, // We only need one result
};

const response = await dynamodb.send(new QueryCommand(queryParams));

if (response.Items && response.Items.length > 0) {
const item = response.Items[0];

// Extract the redirect URL from the item
const redirectUrl = item.redirect?.S;

if (redirectUrl) {
console.log(`Found redirect: ${path} -> ${redirectUrl}`);
return {
status: "302",
statusDescription: "Found",
headers: {
location: [{ key: "Location", value: redirectUrl }],
"cache-control": [
{ key: "Cache-Control", value: `public, max-age=${CACHE_TTL}` },
],
},
};
}
Comment on lines +69 to +103
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify DynamoDB attribute names and consider URL validation.

The query uses slug and access as attribute names. Please ensure these match the actual DynamoDB table schema (primary key and sort key names).

Additionally, the redirect URL extracted from DynamoDB (line 89) is returned without validation. If an attacker could inject malicious URLs into the database, this could pose a security risk.

Consider adding URL validation before redirecting:

if (redirectUrl) {
  // Validate URL format and protocol
  try {
    const url = new URL(redirectUrl);
    if (!['http:', 'https:'].includes(url.protocol)) {
      console.log(`Invalid protocol for redirect URL: ${redirectUrl}`);
      // Fall through to fallback redirect
    } else {
      console.log(`Found redirect: ${path} -> ${redirectUrl}`);
      return {
        status: "302",
        statusDescription: "Found",
        headers: {
          location: [{ key: "Location", value: redirectUrl }],
          "cache-control": [
            { key: "Cache-Control", value: `public, max-age=${CACHE_TTL}` },
          ],
        },
      };
    }
  } catch (e) {
    console.log(`Invalid URL format: ${redirectUrl}`);
    // Fall through to fallback redirect
  }
}

Run the following script to verify the DynamoDB schema:


🏁 Script executed:

#!/bin/bash
# Verify the DynamoDB table schema for infra-core-api-linkry

# Search for the table definition in Terraform or DynamoDB client code
rg -n "infra-core-api-linkry" -A 10 -B 2 --type=tf

# Search for any LinkryRecord or similar type definitions that show the schema
ast-grep --pattern 'interface $_ {
  $$$
  slug: $$$
  $$$
}'

# Search for DynamoDB operations on the linkry table
rg -n "KeyConditionExpression|AttributeName" -A 5 -B 5 | rg -A 10 -B 10 "linkry"

Length of output: 12034


Now I need to check if "slug" and "access" are specifically in the DynamoDB reserved words list. Let me search for that information.

Use ExpressionAttributeNames for attribute names and validate redirect URLs.

Two issues need attention:

  1. Attribute naming inconsistency: The query uses direct attribute names "slug" and "access" in the KeyConditionExpression, but the rest of the linkry codebase consistently uses ExpressionAttributeNames with these same attributes (e.g., src/api/functions/linkry.ts lines 90-94). While "slug" and "access" may not be DynamoDB reserved keywords, using ExpressionAttributeNames when attribute names conflict with reserved words is a defensive best practice throughout your codebase. Update the query to match the codebase pattern:

    KeyConditionExpression: "#slug = :slug AND begins_with(#access, :owner_prefix)",
    ExpressionAttributeNames: {
      "#slug": "slug",
      "#access": "access",
    },
  2. URL validation missing (line 89): The redirect URL is returned without validation. Add protocol and format validation before redirecting to prevent potential security issues from malicious URLs in the database.

🤖 Prompt for AI Agents
In src/linkryEdgeFunction/index.ts around lines 69 to 103, update the
QueryCommandInput to use ExpressionAttributeNames for the attributes used in
KeyConditionExpression (e.g., change to "#slug" and "#access" and add
ExpressionAttributeNames mapping "#slug"->"slug" and "#access"->"access") to
match the codebase pattern and avoid reserved-word issues; after retrieving
item.redirect, validate the URL (ensure it parses, uses an allowed scheme like
https or http, and is a well-formed absolute URL) and only return the 302
response when the validation passes, otherwise log and handle safely (e.g.,
return a 400/404 or skip redirect) to prevent redirecting to malicious or
malformed URLs.

console.log(`Item found but no redirect attribute for path: ${path}`);
} else {
console.log(`No items found for path: ${path}`);
}
} catch (error) {
if (error instanceof Error) {
console.error(
`DynamoDB query failed for ${path} in region ${targetRegion}:`,
error.message,
);
} else {
console.error(`Unexpected error:`, error);
}
}

// Not found - redirect to fallback
return {
status: "307",
statusDescription: "Temporary Redirect",
headers: {
location: [{ key: "Location", value: FALLBACK_URL }],
"cache-control": [
{ key: "Cache-Control", value: `public, max-age=${CACHE_TTL}` },
],
},
};
};
113 changes: 0 additions & 113 deletions src/linkryEdgeFunction/main.py

This file was deleted.

Loading
Loading