-
Notifications
You must be signed in to change notification settings - Fork 1k
server refactor #3893
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v31
Are you sure you want to change the base?
server refactor #3893
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -66,6 +66,9 @@ else | |||||||||||||||||||||
| GHCR_IMAGE="${GHCR_USERNAME}/${GHCR_REPO}:${VERSION_TAG}" | ||||||||||||||||||||||
| fi | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # jwtAudience always matched DOMAIN in the old per-env configs. | ||||||||||||||||||||||
| JWT_AUDIENCE="$DOMAIN" | ||||||||||||||||||||||
|
Comment on lines
+69
to
+70
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate DOMAIN before deriving JWT_AUDIENCE.
Following the existing pattern at lines 58-61 and 87-90, add a validation check for required variables. 🛡️ Suggested validation check+# Validate DOMAIN is set before deriving JWT_AUDIENCE
+if [ -z "$DOMAIN" ]; then
+ echo "Error: DOMAIN not defined in .env file or environment"
+ exit 1
+fi
+
# jwtAudience always matched DOMAIN in the old per-env configs.
JWT_AUDIENCE="$DOMAIN"📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if [ "$HOST" == "staging" ]; then | ||||||||||||||||||||||
| print_header "DEPLOYING TO STAGING HOST" | ||||||||||||||||||||||
| SERVER_HOST=$SERVER_HOST_STAGING | ||||||||||||||||||||||
|
|
@@ -138,6 +141,9 @@ API_KEY=$API_KEY | |||||||||||||||||||||
| DOMAIN=$DOMAIN | ||||||||||||||||||||||
| SUBDOMAIN=$SUBDOMAIN | ||||||||||||||||||||||
| CDN_BASE=$CDN_BASE | ||||||||||||||||||||||
| JWT_AUDIENCE=$JWT_AUDIENCE | ||||||||||||||||||||||
| NUM_WORKERS=$NUM_WORKERS | ||||||||||||||||||||||
| TURNSTILE_SITE_KEY=$TURNSTILE_SITE_KEY | ||||||||||||||||||||||
|
Comment on lines
+144
to
+146
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate required variables before writing to remote environment.
Add validation checks before line 133 to fail fast with a clear error message. 🛡️ Suggested validation checks print_header "EXECUTING UPDATE SCRIPT ON SERVER"
+# Validate required environment variables
+if [ -z "$JWT_AUDIENCE" ]; then
+ echo "Error: JWT_AUDIENCE not defined"
+ exit 1
+fi
+
+if [ -z "$NUM_WORKERS" ]; then
+ echo "Error: NUM_WORKERS not defined in environment"
+ exit 1
+fi
+
+if [ -z "$TURNSTILE_SITE_KEY" ]; then
+ echo "Error: TURNSTILE_SITE_KEY not defined in environment"
+ exit 1
+fi
+
ssh -i $SSH_KEY $REMOTE_USER@$SERVER_HOST "chmod +x $REMOTE_UPDATE_SCRIPT && \🤖 Prompt for AI Agents |
||||||||||||||||||||||
| OTEL_EXPORTER_OTLP_ENDPOINT=$OTEL_EXPORTER_OTLP_ENDPOINT | ||||||||||||||||||||||
| OTEL_AUTH_HEADER=$OTEL_AUTH_HEADER | ||||||||||||||||||||||
| EOL | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| import { JWK } from "jose"; | ||
| import { z } from "zod"; | ||
| import { GameID } from "../core/Schemas"; | ||
| import { simpleHash } from "../core/Util"; | ||
| import { | ||
| GameEnv, | ||
| JwksSchema, | ||
| parseGameEnv, | ||
| } from "../core/configuration/Config"; | ||
|
|
||
| export class ClientEnv { | ||
| private static values: ClientEnvValues | null = null; | ||
| private static publicKey: JWK | null = null; | ||
|
|
||
| /** Test-only. */ | ||
| static reset(): void { | ||
| ClientEnv.values = null; | ||
| ClientEnv.publicKey = null; | ||
| } | ||
|
|
||
| private static get(): ClientEnvValues { | ||
| if (ClientEnv.values) return ClientEnv.values; | ||
| if (typeof window === "undefined") { | ||
| throw new Error("ClientEnv is only available on the browser main thread"); | ||
| } | ||
| const bc = window.BOOTSTRAP_CONFIG; | ||
| if ( | ||
| !bc || | ||
| bc.gameEnv === undefined || | ||
| bc.numWorkers === undefined || | ||
| bc.turnstileSiteKey === undefined || | ||
| bc.jwtAudience === undefined | ||
| ) { | ||
| throw new Error("Missing BOOTSTRAP_CONFIG"); | ||
| } | ||
| if (bc.instanceId === undefined) { | ||
| throw new Error("Missing BOOTSTRAP_CONFIG"); | ||
| } | ||
| ClientEnv.values = { | ||
| gameEnv: parseGameEnv(bc.gameEnv), | ||
| numWorkers: bc.numWorkers, | ||
| turnstileSiteKey: bc.turnstileSiteKey, | ||
| jwtAudience: bc.jwtAudience, | ||
| instanceId: bc.instanceId, | ||
| }; | ||
|
Comment on lines
+27
to
+45
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate Right now Suggested fix if (
!bc ||
bc.gameEnv === undefined ||
bc.numWorkers === undefined ||
bc.turnstileSiteKey === undefined ||
bc.jwtAudience === undefined
) {
throw new Error("Missing BOOTSTRAP_CONFIG");
}
+ if (!Number.isInteger(bc.numWorkers) || bc.numWorkers < 1) {
+ throw new Error(`Invalid BOOTSTRAP_CONFIG.numWorkers: ${bc.numWorkers}`);
+ }
if (bc.instanceId === undefined) {
throw new Error("Missing BOOTSTRAP_CONFIG");
}Also applies to: 94-99 🤖 Prompt for AI Agents |
||
| return ClientEnv.values; | ||
| } | ||
|
|
||
| // TODO: the following methods are duplicated on ServerEnv. The two classes | ||
| // read from different sources (window.BOOTSTRAP_CONFIG vs process.env) but | ||
| // the derived logic is identical. Consolidate into a shared helper that | ||
| // takes a source so we don't have to keep them in sync by hand. | ||
| static env(): GameEnv { | ||
| return ClientEnv.get().gameEnv; | ||
| } | ||
| static numWorkers(): number { | ||
| return ClientEnv.get().numWorkers; | ||
| } | ||
| static turnstileSiteKey(): string { | ||
| return ClientEnv.get().turnstileSiteKey; | ||
| } | ||
| static jwtAudience(): string { | ||
| return ClientEnv.get().jwtAudience; | ||
| } | ||
| static instanceId(): string { | ||
| return ClientEnv.get().instanceId; | ||
| } | ||
| static jwtIssuer(): string { | ||
| const audience = ClientEnv.jwtAudience(); | ||
| return audience === "localhost" | ||
| ? "http://localhost:8787" | ||
| : `https://api.${audience}`; | ||
| } | ||
| static async jwkPublicKey(): Promise<JWK> { | ||
| if (ClientEnv.publicKey) return ClientEnv.publicKey; | ||
| const jwksUrl = ClientEnv.jwtIssuer() + "/.well-known/jwks.json"; | ||
| console.log(`Fetching JWKS from ${jwksUrl}`); | ||
| const response = await fetch(jwksUrl); | ||
| const result = JwksSchema.safeParse(await response.json()); | ||
| if (!result.success) { | ||
| const error = z.prettifyError(result.error); | ||
| console.error("Error parsing JWKS", error); | ||
| throw new Error("Invalid JWKS"); | ||
| } | ||
| ClientEnv.publicKey = result.data.keys[0]; | ||
| return ClientEnv.publicKey; | ||
| } | ||
| static turnIntervalMs(): number { | ||
| return 100; | ||
| } | ||
| static gameCreationRate(): number { | ||
| return ClientEnv.env() === GameEnv.Dev ? 5 * 1000 : 2 * 60 * 1000; | ||
| } | ||
| static workerIndex(gameID: GameID): number { | ||
| return simpleHash(gameID) % ClientEnv.numWorkers(); | ||
| } | ||
| static workerPath(gameID: GameID): string { | ||
| return `w${ClientEnv.workerIndex(gameID)}`; | ||
| } | ||
| } | ||
| /** | ||
| * Values that flow from server → client via index.html. Set on the server from | ||
| * process.env, then re-hydrated on the client from window.BOOTSTRAP_CONFIG. | ||
| */ | ||
|
|
||
| export interface ClientEnvValues { | ||
| gameEnv: GameEnv; | ||
| numWorkers: number; | ||
| turnstileSiteKey: string; | ||
| jwtAudience: string; | ||
| instanceId: string; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: openfrontio/OpenFrontIO
Length of output: 301
🏁 Script executed:
Repository: openfrontio/OpenFrontIO
Length of output: 251
🏁 Script executed:
cat -n .github/workflows/deploy.yml | head -150Repository: openfrontio/OpenFrontIO
Length of output: 7189
🏁 Script executed:
Repository: openfrontio/OpenFrontIO
Length of output: 93
🏁 Script executed:
Repository: openfrontio/OpenFrontIO
Length of output: 6158
🏁 Script executed:
Repository: openfrontio/OpenFrontIO
Length of output: 3070
Add validation for NUM_WORKERS and TURNSTILE_SITE_KEY in deploy.sh.
These variables are passed to the deployment script but are not validated before use. If
vars.NUM_WORKERSorvars.TURNSTILE_SITE_KEYare not defined in repository settings, empty strings will be passed to the remote server, which may cause the application to fail at runtime even though the deployment appears to succeed.Add validation checks similar to the existing GHCR_USERNAME check (line 58-60 in deploy.sh):
🤖 Prompt for AI Agents