Skip to content

init target variables #166

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Oct 24, 2024
Merged

init target variables #166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5cab4f0
init target variables
jsbroks Oct 24, 2024
cab9e82
add missing imports
jsbroks Oct 24, 2024
dcf4316
add validation skipping
jsbroks Oct 24, 2024
068e78a
Fix vars
jsbroks Oct 24, 2024
3625589
clean up
jsbroks Oct 24, 2024
d5adfe2
clean up
jsbroks Oct 24, 2024
39a7cb6
clean up endpoints
jsbroks Oct 24, 2024
6de6ffa
add objects
jsbroks Oct 24, 2024
21895c6
add more validation
jsbroks Oct 24, 2024
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
19 changes: 6 additions & 13 deletions .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
# Since .env is gitignored, you can use .env.example to build a new `.env` file when you clone the repo.
# Keep this file up-to-date when you add new variables to \`.env\`.
BASE_URL="http://localhost:3000"
POSTGRES_URL="postgresql://ctrlplane:ctrlplane@127.0.0.1:5432/ctrlplane"
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 24, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security: Remove credentials from example PostgreSQL URL.

The PostgreSQL URL contains default credentials which could be accidentally used in production. Consider using placeholder values instead.

-POSTGRES_URL="postgresql://ctrlplane:ctrlplane@127.0.0.1:5432/ctrlplane"
+POSTGRES_URL="postgresql://<username>:<password>@127.0.0.1:5432/<database>"
📝 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
POSTGRES_URL="postgresql://ctrlplane:ctrlplane@127.0.0.1:5432/ctrlplane"
POSTGRES_URL="postgresql://<username>:<password>@127.0.0.1:5432/<database>"

REDIS_URL="redis://127.0.0.1:6379"

# This file will be committed to version control, so make sure not to have any secrets in it.
# If you are cloning this repo, create a copy of this file named `.env` and populate it with your secrets.

# The database URL is used to connect to your Supabase database.
POSTGRES_URL="postgres://postgres.[USERNAME]:[PASSWORD]@aws-0-eu-central-1.pooler.supabase.com:6543/postgres?workaround=supabase-pooler.vercel"

# You can generate the secret via 'openssl rand -base64 32' on Unix
# @see https://next-auth.js.org/configuration/options#secret
AUTH_SECRET='supersecret'
AUTH_TRUST_HOST=false
Comment on lines 4 to +6
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 24, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security: Add warning about AUTH_SECRET configuration.

The example includes a weak default secret that could be accidentally used in production. Add a warning and use a placeholder instead.

-AUTH_SECRET='supersecret'
+# WARNING: Generate a strong random secret in production!
+# Minimum length: 32 characters
+AUTH_SECRET='<your-secure-random-secret>'
 AUTH_TRUST_HOST=false
📝 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
# This file will be committed to version control, so make sure not to have any secrets in it.
# If you are cloning this repo, create a copy of this file named `.env` and populate it with your secrets.
# The database URL is used to connect to your Supabase database.
POSTGRES_URL="postgres://postgres.[USERNAME]:[PASSWORD]@aws-0-eu-central-1.pooler.supabase.com:6543/postgres?workaround=supabase-pooler.vercel"
# You can generate the secret via 'openssl rand -base64 32' on Unix
# @see https://next-auth.js.org/configuration/options#secret
AUTH_SECRET='supersecret'
AUTH_TRUST_HOST=false
# WARNING: Generate a strong random secret in production!
# Minimum length: 32 characters
AUTH_SECRET='<your-secure-random-secret>'
AUTH_TRUST_HOST=false


VARIABLES_AES_256_KEY=0000000000000000000000000000000000000000000000000000000000000000
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 24, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security: Replace example encryption key with placeholder.

The AES key is currently set to all zeros, which is insecure. Replace with a placeholder and add guidance on generating a secure key.

-VARIABLES_AES_256_KEY=0000000000000000000000000000000000000000000000000000000000000000
+# AES-256 encryption key for variable encryption (required)
+# Generate a secure 64-character hex key for production use
+# Example command: openssl rand -hex 32
+VARIABLES_AES_256_KEY=<generated-hex-key>
📝 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
VARIABLES_AES_256_KEY=0000000000000000000000000000000000000000000000000000000000000000
# AES-256 encryption key for variable encryption (required)
# Generate a secure 64-character hex key for production use
# Example command: openssl rand -hex 32
VARIABLES_AES_256_KEY=<generated-hex-key>


Comment on lines +1 to 9
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 24, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the search results, I can now provide a comprehensive response about the environment variables.

The .env.example file is missing some required environment variables.

The following required variables should be added to the .env.example file:

  • GITHUB_BOT_NAME - GitHub bot name (optional)
  • GITHUB_BOT_CLIENT_ID - GitHub bot client ID (optional)
  • GITHUB_BOT_CLIENT_SECRET - GitHub bot client secret (optional)
  • GITHUB_BOT_APP_ID - GitHub bot app ID (optional)
  • GITHUB_BOT_PRIVATE_KEY - GitHub bot private key (optional)
  • GITHUB_WEBHOOK_SECRET - GitHub webhook secret (optional)
  • GITHUB_URL - GitHub URL (optional)
  • OTEL_SAMPLER_RATIO - OpenTelemetry sampling ratio (optional, defaults to 1)

These variables are defined in the environment schema (apps/webservice/src/env.ts) but missing from the example configuration file.

🔗 Analysis chain

Add missing configuration variables.

Based on the AI summary, this PR introduces new functionality for target variables and secrets management. However, some potentially required variables might be missing from the example configuration.

Let's check for any missing required environment variables:

Let me gather more information about environment variables by checking the schema definitions and configuration files.

Let me search for all environment variable schema definitions to identify required variables.

Let me try a different search pattern to find environment variable definitions.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for environment variable usage in the codebase
# Look for process.env references in TypeScript/JavaScript files
rg -t ts -t js 'process\.env\.' -A 1

# Look for @env decorator usage in TypeScript files
rg -t ts '@env' -A 1

Length of output: 3847

Script:

#!/bin/bash
# Search for environment schema definitions
rg -t ts "z.object\({" -A 20

# Look for environment variable validation
fd "env" --type f --exec cat {}

Length of output: 67900

Script:

#!/bin/bash
# Search for environment variable schema definitions in zod objects
rg -t ts "createEnv\({" -B 2 -A 20

Length of output: 124

Script:

#!/bin/bash
# Search for environment variable schemas
rg -t ts "server: \{" -B 2 -A 20

# Also search for any environment variable validation
rg -t ts "z\.string\(\)\.min\(1\)" -B 2 -A 2

Length of output: 34031

JOB_AGENT_WORKSPACE="ctrlplane"
JOB_AGENT_NAME="agent"
JOB_AGENT_API_KEY=
1 change: 1 addition & 0 deletions apps/event-worker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ COPY packages/db/package.json ./packages/db/package.json
COPY packages/validators/package.json ./packages/validators/package.json
COPY packages/logger/package.json ./packages/logger/package.json
COPY packages/job-dispatch/package.json ./packages/job-dispatch/package.json
COPY packages/secrets/package.json ./packages/secrets/package.json
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 24, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Environment variable VARIABLES_AES_256_KEY required for secrets package

The secrets package implements AES-256 encryption and requires a specific environment variable to be set:

  • VARIABLES_AES_256_KEY: A 64-character hexadecimal string (32 bytes / 256 bits) required for encryption/decryption operations
  • The package uses @t3-oss/env-core for environment variable validation
  • The encryption service is properly implemented with secure defaults (AES-256-CBC with random IV)

Make sure to set the required environment variable in your event-worker service configuration.

🔗 Analysis chain

LGTM! Verify secrets package configuration.

The addition of the secrets package follows the established pattern for shared packages. The package is correctly copied before dependency installation.

Let's verify the secrets package setup:

Let me try to gather more information about the secrets package to better understand its configuration and requirements.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the secrets package requires specific environment variables
rg -g 'packages/secrets/**' 'process\.env\.' -A 2

# Check for any security-related configuration files
fd -g 'packages/secrets/**' -e config.ts -e config.js

Length of output: 107

Script:

#!/bin/bash
# Check the contents of the secrets package
fd . packages/secrets -t f -x cat {}

# Look for any environment variable patterns in a more generic way
rg -g 'packages/secrets/**' -i '(env|secret|config|environment)' -A 2

# Check package.json for dependencies and scripts
cat packages/secrets/package.json

Length of output: 5895


COPY apps/event-worker/package.json ./apps/event-worker/package.json

Expand Down
1 change: 1 addition & 0 deletions apps/jobs/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ COPY packages/db/package.json ./packages/db/package.json
COPY packages/validators/package.json ./packages/validators/package.json
COPY packages/logger/package.json ./packages/logger/package.json
COPY packages/job-dispatch/package.json ./packages/job-dispatch/package.json
COPY packages/secrets/package.json ./packages/secrets/package.json

COPY apps/jobs/package.json ./apps/jobs/package.json

Expand Down
1 change: 1 addition & 0 deletions apps/webservice/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
COPY packages/job-dispatch/package.json ./packages/job-dispatch/package.json
COPY packages/ui/package.json ./packages/ui/package.json
COPY packages/logger/package.json ./packages/logger/package.json
COPY packages/secrets/package.json ./packages/secrets/package.json

COPY apps/webservice/package.json ./apps/webservice/package.json

Expand All @@ -56,7 +57,7 @@
EXPOSE 3000

ENV PORT=3000
ENV AUTH_TRUST_HOST=true

Check warning on line 60 in apps/webservice/Dockerfile

View workflow job for this annotation

GitHub Actions / build (linux/amd64)

Sensitive data should not be used in the ARG or ENV commands 

SecretsUsedInArgOrEnv: Do not use ARG or ENV instructions for sensitive data (ENV "AUTH_TRUST_HOST")
More info: https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/
ENV NODE_ENV=production
ENV HOSTNAME=0.0.0.0

Expand Down
28 changes: 26 additions & 2 deletions apps/webservice/src/app/api/v1/target-providers/[providerId]/set/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,22 @@ const bodySchema = z.object({
targets: z.array(
createTarget
.omit({ lockedAt: true, providerId: true, workspaceId: true })
.extend({ metadata: z.record(z.string()).optional() }),
.extend({
metadata: z.record(z.string()).optional(),
variables: z
.array(
z.object({
key: z.string(),
value: z.union([z.string(), z.number(), z.boolean(), z.null()]),
sensitive: z.boolean(),
}),
)
.optional()
.refine(
(vars) => new Set(vars?.map((v) => v.key)).size === vars?.length,
"Duplicate variable keys are not allowed",
),
}),
),
});

Expand Down Expand Up @@ -56,7 +71,16 @@ export const PATCH = async (
workspaceId: provider.workspaceId,
}));

const targets = await upsertTargets(db, targetsToInsert);
const targets = await upsertTargets(
db,
targetsToInsert.map((t) => ({
...t,
variables: t.variables?.map((v) => ({
...v,
value: v.value ?? null,
})),
})),
);

return NextResponse.json({ targets });
};
26 changes: 24 additions & 2 deletions apps/webservice/src/app/api/v1/workspaces/[workspaceId]/targets/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,22 @@ export const GET = async (
const bodySchema = z.array(
createTarget
.omit({ lockedAt: true, providerId: true, workspaceId: true })
.extend({ metadata: z.record(z.string()).optional() }),
.extend({
metadata: z.record(z.string()).optional(),
variables: z
.array(
z.object({
key: z.string(),
value: z.union([z.string(), z.number(), z.boolean(), z.null()]),
sensitive: z.boolean(),
}),
)
.optional()
.refine(
(vars) => new Set(vars?.map((v) => v.key)).size === vars?.length,
"Duplicate variable keys are not allowed",
),
}),
);

const canUpsertTarget = async (userId: string, workspaceId: string) =>
Expand Down Expand Up @@ -168,7 +183,14 @@ export const PATCH = async (

const targets = await upsertTargets(
db,
parsedTargets.map((t) => ({ ...t, workspaceId })),
parsedTargets.map((t) => ({
...t,
workspaceId,
variables: t.variables?.map((v) => ({
...v,
value: v.value ?? null,
})),
})),
);

return NextResponse.json({ count: targets.length });
Expand Down
2 changes: 1 addition & 1 deletion packages/api/src/router/system.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const systemRouter = createTRPCRouter({
.array(
z.object({
key: z.enum(["name", "slug"]),
value: z.any(),
value: z.string(),
}),
)
.optional(),
Expand Down
15 changes: 15 additions & 0 deletions packages/db/drizzle/0021_first_ikaris.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
CREATE TABLE IF NOT EXISTS "target_variable" (
"id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL,
"target_id" uuid NOT NULL,
"key" text NOT NULL,
"value" jsonb NOT NULL,
"sensitive" boolean DEFAULT false NOT NULL
);
Comment on lines +1 to +7
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 24, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding constraints for data integrity.

While the basic structure is sound, consider adding these constraints for better data integrity:

  1. Add a length limit to the key field (e.g., text(100)) to prevent excessive storage
  2. Add a CHECK constraint to validate the JSONB structure
 CREATE TABLE IF NOT EXISTS "target_variable" (
 	"id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL,
 	"target_id" uuid NOT NULL,
-	"key" text NOT NULL,
+	"key" text NOT NULL CHECK (length(key) <= 100),
 	"value" jsonb NOT NULL,
 	"sensitive" boolean DEFAULT false NOT NULL
+	CONSTRAINT "value_is_valid" CHECK (jsonb_typeof(value) IN ('object', 'array', 'string', 'number', 'boolean'))
 );
📝 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
CREATE TABLE IF NOT EXISTS "target_variable" (
"id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL,
"target_id" uuid NOT NULL,
"key" text NOT NULL,
"value" jsonb NOT NULL,
"sensitive" boolean DEFAULT false NOT NULL
);
CREATE TABLE IF NOT EXISTS "target_variable" (
"id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL,
"target_id" uuid NOT NULL,
"key" text NOT NULL CHECK (length(key) <= 100),
"value" jsonb NOT NULL,
"sensitive" boolean DEFAULT false NOT NULL
CONSTRAINT "value_is_valid" CHECK (jsonb_typeof(value) IN ('object', 'array', 'string', 'number', 'boolean'))
);

--> statement-breakpoint
DO $$ BEGIN
ALTER TABLE "target_variable" ADD CONSTRAINT "target_variable_target_id_target_id_fk" FOREIGN KEY ("target_id") REFERENCES "public"."target"("id") ON DELETE cascade ON UPDATE no action;
EXCEPTION
WHEN duplicate_object THEN null;
END $$;
--> statement-breakpoint
CREATE UNIQUE INDEX IF NOT EXISTS "target_variable_target_id_key_index" ON "target_variable" USING btree ("target_id","key");
Loading
Loading