Skip to content

feat: env var secret delivery — stop inlining credentials in YAML #23

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
TerrifiedBug merged 5 commits into from
Mar 6, 2026
Merged

feat: env var secret delivery — stop inlining credentials in YAML #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bcec861
feat: add convertSecretRefsToEnvVars for env var interpolation
TerrifiedBug Mar 6, 2026
733ed54
feat: use env var placeholders instead of inlining secrets in YAML
TerrifiedBug Mar 6, 2026
6bc5a8e
fix: normalize secret names to valid env var format
TerrifiedBug Mar 6, 2026
142fc87
fix: include secrets in checksum and warn on name collisions
TerrifiedBug Mar 6, 2026
8fae4a1
fix: hoist secrets query outside pipeline loop and add deterministic …
TerrifiedBug Mar 6, 2026
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
46 changes: 29 additions & 17 deletions src/app/api/agent/config/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { NextResponse } from "next/server";
import yaml from "js-yaml";
import { prisma } from "@/lib/prisma";
import { authenticateAgent } from "@/server/services/agent-auth";
import { resolveSecretRefs, resolveCertRefs } from "@/server/services/secret-resolver";
import { convertSecretRefsToEnvVars, resolveCertRefs, secretNameToEnvVar } from "@/server/services/secret-resolver";
import { decrypt } from "@/server/services/crypto";
import { createHash } from "crypto";

Expand Down Expand Up @@ -56,47 +56,59 @@ export async function GET(request: Request) {
const pipelineConfigs = [];
const certBasePath = "/var/lib/vf-agent/certs";

// Pre-resolve all environment secrets once (shared across all pipelines)
const secrets: Record<string, string> = {};
if (environment.secretBackend === "BUILTIN") {
const envSecrets = await prisma.secret.findMany({
where: { environmentId: agent.environmentId },
orderBy: { name: "asc" },
});
for (const s of envSecrets) {
const envKey = secretNameToEnvVar(s.name);
if (secrets[envKey] !== undefined) {
console.warn(`[agent-config] Secret name collision: "${s.name}" normalizes to "${envKey}" which is already set`);
}
secrets[envKey] = decrypt(s.encryptedValue);
}
}
Comment on lines +62 to +73
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

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

Secret names can normalize to colliding env var keys — for example, both db-password and db_password normalize to VF_SECRET_DB_PASSWORD. While the deterministic orderBy: { name: "asc" } makes the collision resolution predictable, one secret's decrypted value will silently overwrite the other in the secrets dict, and both SECRET[db-password] and SECRET[db_password] references in the config will emit the same placeholder. The agent will set the wrong credential without any error.

Consider validating at secret-creation time that no two secrets in the same environment normalize to the same env var key, rejecting new secrets that would create collisions.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/app/api/agent/config/route.ts
Line: 62-73

Comment:
Secret names can normalize to colliding env var keys — for example, both `db-password` and `db_password` normalize to `VF_SECRET_DB_PASSWORD`. While the deterministic `orderBy: { name: "asc" }` makes the collision resolution predictable, one secret's decrypted value will silently overwrite the other in the `secrets` dict, and both `SECRET[db-password]` and `SECRET[db_password]` references in the config will emit the same placeholder. The agent will set the wrong credential without any error.

Consider validating at secret-creation time that no two secrets in the same environment normalize to the same env var key, rejecting new secrets that would create collisions.

How can I resolve this? If you propose a fix, please make it concise.


for (const pipeline of pipelines) {
try {
const latestVersion = pipeline.versions[0];
if (!latestVersion?.configYaml) continue; // no deployed version yet

const version = latestVersion.version;
let configYaml = latestVersion.configYaml;
const secrets: Record<string, string> = {};
let certFiles: Array<{ name: string; filename: string; data: string }> = [];

if (environment.secretBackend === "BUILTIN") {
// Parse versioned YAML back to objects so we can resolve SECRET[]/CERT[]
// references at the object level. This ensures js-yaml properly quotes
// values containing special characters when we re-dump.
// Parse versioned YAML back to objects so we can resolve references
// at the object level. This ensures js-yaml properly quotes values
// containing special characters when we re-dump.
const parsedConfig = yaml.load(configYaml) as Record<string, unknown>;

// Walk config objects and resolve all SECRET[name] → actual values
const withSecrets = await resolveSecretRefs(parsedConfig, agent.environmentId);
// Convert SECRET[name] → ${VF_SECRET_NAME} env var placeholders.
// Vector interpolates these from environment variables set by the agent.
const withEnvVars = convertSecretRefsToEnvVars(parsedConfig);

// Walk config objects and resolve all CERT[name] → deploy file paths
const { config: withCerts, certFiles: certs } = await resolveCertRefs(
withSecrets,
withEnvVars,
agent.environmentId,
certBasePath,
);
certFiles = certs;

// Re-dump to YAML with proper quoting for special characters
configYaml = yaml.dump(withCerts, { indent: 2, lineWidth: -1, noRefs: true });

// Deliver all environment secrets as env vars
const envSecrets = await prisma.secret.findMany({
where: { environmentId: agent.environmentId },
});
for (const s of envSecrets) {
secrets[`VF_SECRET_${s.name}`] = decrypt(s.encryptedValue);
}
}
// External backend: configYaml is used as-is with references intact

const checksum = createHash("sha256").update(configYaml).digest("hex");
// Include secrets in checksum so secret rotation triggers agent restart
const checksumInput = Object.keys(secrets).length > 0
? configYaml + JSON.stringify(secrets, Object.keys(secrets).sort())
: configYaml;
const checksum = createHash("sha256").update(checksumInput).digest("hex");

pipelineConfigs.push({
pipelineId: pipeline.id,
Expand Down
43 changes: 43 additions & 0 deletions src/server/services/secret-resolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,49 @@ export async function resolveCertRefs(
return { config: resolved, certFiles };
}

/**
* Normalize a secret name to a valid environment variable name.
* Replaces non-alphanumeric characters with underscores and uppercases.
* e.g. "db-password" → "DB_PASSWORD", "my.api.key" → "MY_API_KEY"
*/
export function secretNameToEnvVar(name: string): string {
return `VF_SECRET_${name.replace(/[^a-zA-Z0-9]/g, "_").toUpperCase()}`;
}
Comment on lines +101 to +103
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

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

Name-collision risk after normalization

secretNameToEnvVar maps multiple distinct secret names to the same env var key. For example, db-password and db_password both normalize to VF_SECRET_DB_PASSWORD. If both secrets exist in the same environment, the secrets dict in route.ts (built by iterating envSecrets in an unspecified order) will silently overwrite one value with the other. At the same time, both SECRET[db-password] and SECRET[db_password] in the pipeline YAML would emit the identical placeholder ${VF_SECRET_DB_PASSWORD}, so the wrong credential would be injected at runtime with no error surfaced.

Since the YAML placeholder and the delivered env var key must agree, the normalization itself is correct — but it creates an implicit constraint that DB-level uniqueness (environmentId + name) no longer guarantees uniqueness of the resolved key. Consider adding a uniqueness check at secret-creation time (or during config delivery) that rejects a new secret whose normalized name collides with an existing one.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/server/services/secret-resolver.ts
Line: 101-103

Comment:
**Name-collision risk after normalization**

`secretNameToEnvVar` maps multiple distinct secret names to the same env var key. For example, `db-password` and `db_password` both normalize to `VF_SECRET_DB_PASSWORD`. If both secrets exist in the same environment, the `secrets` dict in `route.ts` (built by iterating `envSecrets` in an unspecified order) will silently overwrite one value with the other. At the same time, both `SECRET[db-password]` and `SECRET[db_password]` in the pipeline YAML would emit the identical placeholder `${VF_SECRET_DB_PASSWORD}`, so the wrong credential would be injected at runtime with no error surfaced.

Since the YAML placeholder and the delivered env var key must agree, the normalization itself is correct — but it creates an implicit constraint that DB-level uniqueness (`environmentId + name`) no longer guarantees uniqueness of the resolved key. Consider adding a uniqueness check at secret-creation time (or during config delivery) that rejects a new secret whose normalized name collides with an existing one.

How can I resolve this? If you propose a fix, please make it concise.


/**
* Walk a config object and convert SECRET[name] references to
* ${VF_SECRET_NAME} env var placeholders for Vector interpolation.
* Pure string transformation — no DB lookups or decryption.
*/
export function convertSecretRefsToEnvVars(
config: Record<string, unknown>,
): Record<string, unknown> {
return walkConvertSecretRefs(config);
Comment on lines +110 to +113
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

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

The new convertSecretRefsToEnvVars function is a pure string transformation that blindly converts any SECRET[name] reference to an env var placeholder regardless of whether that secret actually exists in the database.

If a pipeline config references a secret that doesn't exist (e.g., a typo or deleted secret), the YAML will silently contain ${VF_SECRET_MISSING_NAME}. Vector will then resolve this to an empty string or fail to start with an unclear error message.

Unlike the old resolveSecretRefs which validated references at delivery time (throwing "Secret not found" errors), there is now no fail-fast validation. Consider either:

  1. Adding validation in route.ts to check that all referenced secrets exist before converting to placeholders, or
  2. Including a validation step in convertSecretRefsToEnvVars that collects and validates references
Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/server/services/secret-resolver.ts
Line: 110-113

Comment:
The new `convertSecretRefsToEnvVars` function is a pure string transformation that blindly converts any `SECRET[name]` reference to an env var placeholder regardless of whether that secret actually exists in the database.

If a pipeline config references a secret that doesn't exist (e.g., a typo or deleted secret), the YAML will silently contain `${VF_SECRET_MISSING_NAME}`. Vector will then resolve this to an empty string or fail to start with an unclear error message.

Unlike the old `resolveSecretRefs` which validated references at delivery time (throwing "Secret not found" errors), there is now no fail-fast validation. Consider either:
1. Adding validation in `route.ts` to check that all referenced secrets exist before converting to placeholders, or
2. Including a validation step in `convertSecretRefsToEnvVars` that collects and validates references

How can I resolve this? If you propose a fix, please make it concise.

}
Comment on lines +110 to +114
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

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

convertSecretRefsToEnvVars performs a pure string transformation without validating that referenced secrets exist. If a pipeline config contains SECRET[missing-name] but that secret doesn't exist in the database, the YAML will be emitted with the literal placeholder ${VF_SECRET_MISSING_NAME}. Vector will fail to interpolate it (resulting in an empty string or startup failure), but the error won't be traced back to a missing secret — it will appear as a runtime failure on the agent instead of a validation error at deploy time.

Consider adding validation during config delivery (in route.ts) to ensure all SECRET[name] references in the config have matching secrets before converting to placeholders.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/server/services/secret-resolver.ts
Line: 110-114

Comment:
`convertSecretRefsToEnvVars` performs a pure string transformation without validating that referenced secrets exist. If a pipeline config contains `SECRET[missing-name]` but that secret doesn't exist in the database, the YAML will be emitted with the literal placeholder `${VF_SECRET_MISSING_NAME}`. Vector will fail to interpolate it (resulting in an empty string or startup failure), but the error won't be traced back to a missing secret — it will appear as a runtime failure on the agent instead of a validation error at deploy time.

Consider adding validation during config delivery (in `route.ts`) to ensure all `SECRET[name]` references in the config have matching secrets before converting to placeholders.

How can I resolve this? If you propose a fix, please make it concise.


function walkConvertSecretRefs(
obj: Record<string, unknown>,
): Record<string, unknown> {
const result: Record<string, unknown> = {};

for (const [key, value] of Object.entries(obj)) {
if (typeof value === "string") {
const match = value.match(SECRET_REF_PATTERN);
if (match) {
result[key] = `\${${secretNameToEnvVar(match[1])}}`;
} else {
result[key] = value;
}
} else if (typeof value === "object" && value !== null && !Array.isArray(value)) {
result[key] = walkConvertSecretRefs(value as Record<string, unknown>);
} else {
result[key] = value;
Comment on lines +129 to +132
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

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

walkConvertSecretRefs skips arrays (due to the !Array.isArray(value) guard) and passes them through unchanged. If a Vector config field contains an array with a string element matching SECRET[name], that reference will not be converted to the ${VF_SECRET_NAME} placeholder — it will be emitted verbatim as SECRET[name]. Vector will then treat it as a literal string value rather than a reference to interpolate from an env var, causing the feature relying on that secret to malfunction.

Most Vector credential fields are object properties (not arrays), so real-world impact is limited. However, the conversion logic should be complete:

} else if (Array.isArray(value)) {
  result[key] = value.map((item) =>
    typeof item === "string"
      ? (item.match(SECRET_REF_PATTERN)
          ? `\${${secretNameToEnvVar(item.match(SECRET_REF_PATTERN)![1])}}`
          : item)
      : typeof item === "object" && item !== null
        ? walkConvertSecretRefs(item as Record<string, unknown>)
        : item
  );
} else if (typeof value === "object" && value !== null) {
Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/server/services/secret-resolver.ts
Line: 129-132

Comment:
`walkConvertSecretRefs` skips arrays (due to the `!Array.isArray(value)` guard) and passes them through unchanged. If a Vector config field contains an array with a string element matching `SECRET[name]`, that reference will not be converted to the `${VF_SECRET_NAME}` placeholder — it will be emitted verbatim as `SECRET[name]`. Vector will then treat it as a literal string value rather than a reference to interpolate from an env var, causing the feature relying on that secret to malfunction.

Most Vector credential fields are object properties (not arrays), so real-world impact is limited. However, the conversion logic should be complete:

```ts
} else if (Array.isArray(value)) {
  result[key] = value.map((item) =>
    typeof item === "string"
      ? (item.match(SECRET_REF_PATTERN)
          ? `\${${secretNameToEnvVar(item.match(SECRET_REF_PATTERN)![1])}}`
          : item)
      : typeof item === "object" && item !== null
        ? walkConvertSecretRefs(item as Record<string, unknown>)
        : item
  );
} else if (typeof value === "object" && value !== null) {
```

How can I resolve this? If you propose a fix, please make it concise.

}
}

return result;
}
Comment on lines +110 to +137
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

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

Referenced secrets are no longer validated at delivery time

The old resolveSecretRefs explicitly verified that every SECRET[name] reference in the config had a corresponding secret in the environment, throwing a descriptive error if one was missing:

for (const ref of refs) {
  if (!secretMap.has(ref)) {
    throw new Error(`Secret "${ref}" not found in environment`);
  }
}

The new convertSecretRefsToEnvVars is a pure string transformation — it blindly converts any SECRET[name] reference to an env var placeholder regardless of whether that secret exists in the database. If a pipeline config references a secret that doesn't exist (e.g., a typo, or a secret that was deleted), the YAML will silently contain ${VF_SECRET_MISSING_NAME}. Vector will resolve this to an empty string or fail to start, producing a runtime error on the agent that's difficult to trace back to a missing secret.

Since the secrets dict is built from all environment secrets anyway (in route.ts lines 91–96), it would be straightforward to check that every SECRET[...] reference found in the config has a matching key in the secrets before returning the config response, and return an error if any are missing.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/server/services/secret-resolver.ts
Line: 101-128

Comment:
**Referenced secrets are no longer validated at delivery time**

The old `resolveSecretRefs` explicitly verified that every `SECRET[name]` reference in the config had a corresponding secret in the environment, throwing a descriptive error if one was missing:

```ts
for (const ref of refs) {
  if (!secretMap.has(ref)) {
    throw new Error(`Secret "${ref}" not found in environment`);
  }
}
```

The new `convertSecretRefsToEnvVars` is a pure string transformation — it blindly converts any `SECRET[name]` reference to an env var placeholder regardless of whether that secret exists in the database. If a pipeline config references a secret that doesn't exist (e.g., a typo, or a secret that was deleted), the YAML will silently contain `${VF_SECRET_MISSING_NAME}`. Vector will resolve this to an empty string or fail to start, producing a runtime error on the agent that's difficult to trace back to a missing secret.

Since the `secrets` dict is built from all environment secrets anyway (in `route.ts` lines 91–96), it would be straightforward to check that every `SECRET[...]` reference found in the config has a matching key in the secrets before returning the config response, and return an error if any are missing.

How can I resolve this? If you propose a fix, please make it concise.


/* ------------------------------------------------------------------ */
/* Helpers */
/* ------------------------------------------------------------------ */
Expand Down
Loading