From 9f98ccca5893bd7c427b4c88f8a202165c004f8f Mon Sep 17 00:00:00 2001 From: Jiayuan Zhang Date: Sun, 15 Feb 2026 04:23:56 +0800 Subject: [PATCH] feat(skills): store API keys in per-skill .env files Move skill environment variables from centralized skills.env.json5 to per-skill .env files within each skill's directory. This makes credential management more intuitive and self-contained. - Fix parser to handle metadata.requires, always, os, skillKey, install - Add minimal .env parser (dotenv.ts) and load .env at skill parse time - Add env field to Skill type for per-skill environment variables - Update eligibility checker to use skill.env instead of CredentialManager - Preserve user .env files across bundled skill upgrades in loader Co-Authored-By: Claude Opus 4.6 --- packages/core/src/agent/skills/dotenv.test.ts | 58 ++++++++ packages/core/src/agent/skills/dotenv.ts | 26 ++++ .../core/src/agent/skills/eligibility.test.ts | 34 +++++ packages/core/src/agent/skills/eligibility.ts | 22 +-- packages/core/src/agent/skills/index.ts | 1 + packages/core/src/agent/skills/loader.ts | 20 ++- packages/core/src/agent/skills/parser.test.ts | 129 +++++++++++++++++- packages/core/src/agent/skills/parser.ts | 59 ++++++-- packages/core/src/agent/skills/types.ts | 2 + 9 files changed, 323 insertions(+), 28 deletions(-) create mode 100644 packages/core/src/agent/skills/dotenv.test.ts create mode 100644 packages/core/src/agent/skills/dotenv.ts diff --git a/packages/core/src/agent/skills/dotenv.test.ts b/packages/core/src/agent/skills/dotenv.test.ts new file mode 100644 index 00000000..0639b901 --- /dev/null +++ b/packages/core/src/agent/skills/dotenv.test.ts @@ -0,0 +1,58 @@ +import { describe, it, expect } from "vitest"; +import { parseDotEnv } from "./dotenv.js"; + +describe("parseDotEnv", () => { + it("should parse basic KEY=VALUE pairs", () => { + const result = parseDotEnv("FOO=bar\nBAZ=qux"); + expect(result).toEqual({ FOO: "bar", BAZ: "qux" }); + }); + + it("should handle double-quoted values", () => { + const result = parseDotEnv('KEY="hello world"'); + expect(result).toEqual({ KEY: "hello world" }); + }); + + it("should handle single-quoted values", () => { + const result = parseDotEnv("KEY='hello world'"); + expect(result).toEqual({ KEY: "hello world" }); + }); + + it("should skip comments", () => { + const result = parseDotEnv("# This is a comment\nKEY=value\n# Another comment"); + expect(result).toEqual({ KEY: "value" }); + }); + + it("should skip blank lines", () => { + const result = parseDotEnv("\n\nKEY=value\n\n"); + expect(result).toEqual({ KEY: "value" }); + }); + + it("should handle empty values", () => { + const result = parseDotEnv("KEY="); + expect(result).toEqual({ KEY: "" }); + }); + + it("should handle equals sign in value", () => { + const result = parseDotEnv("KEY=foo=bar=baz"); + expect(result).toEqual({ KEY: "foo=bar=baz" }); + }); + + it("should handle spaces around key and value", () => { + const result = parseDotEnv(" KEY = value "); + expect(result).toEqual({ KEY: "value" }); + }); + + it("should skip lines without equals sign", () => { + const result = parseDotEnv("INVALID_LINE\nKEY=value"); + expect(result).toEqual({ KEY: "value" }); + }); + + it("should return empty object for empty content", () => { + expect(parseDotEnv("")).toEqual({}); + }); + + it("should handle CRLF line endings", () => { + const result = parseDotEnv("A=1\r\nB=2\r\n"); + expect(result).toEqual({ A: "1", B: "2" }); + }); +}); diff --git a/packages/core/src/agent/skills/dotenv.ts b/packages/core/src/agent/skills/dotenv.ts new file mode 100644 index 00000000..54b12ca9 --- /dev/null +++ b/packages/core/src/agent/skills/dotenv.ts @@ -0,0 +1,26 @@ +/** + * Minimal .env file parser + * + * Supports KEY=VALUE, KEY="VALUE", KEY='VALUE', # comments, empty lines. + */ + +export function parseDotEnv(content: string): Record { + const result: Record = {}; + for (const line of content.split("\n")) { + const trimmed = line.trim(); + if (!trimmed || trimmed.startsWith("#")) continue; + const eqIndex = trimmed.indexOf("="); + if (eqIndex === -1) continue; + const key = trimmed.slice(0, eqIndex).trim(); + let value = trimmed.slice(eqIndex + 1).trim(); + // Strip surrounding quotes + if ( + (value.startsWith('"') && value.endsWith('"')) || + (value.startsWith("'") && value.endsWith("'")) + ) { + value = value.slice(1, -1); + } + if (key) result[key] = value; + } + return result; +} diff --git a/packages/core/src/agent/skills/eligibility.test.ts b/packages/core/src/agent/skills/eligibility.test.ts index 8d888abb..f4bd744c 100644 --- a/packages/core/src/agent/skills/eligibility.test.ts +++ b/packages/core/src/agent/skills/eligibility.test.ts @@ -7,6 +7,7 @@ function createSkill( id: string, frontmatter: Partial & { name: string }, source: "bundled" | "profile" = "bundled", + env: Record = {}, ): Skill { return { id, @@ -14,6 +15,7 @@ function createSkill( instructions: "Test instructions", source, filePath: `/path/to/${id}/SKILL.md`, + env, }; } @@ -236,6 +238,38 @@ describe("eligibility", () => { expect(result.eligible).toBe(true); }); + it("should be eligible when env var is in skill .env file", () => { + delete process.env.SKILL_API_KEY; + + const skill = createSkill("test", { + name: "Test Skill", + metadata: { + requires: { + env: ["SKILL_API_KEY"], + }, + }, + }, "bundled", { SKILL_API_KEY: "test-value" }); + + const result = checkEligibility(skill, ctx("darwin")); + expect(result.eligible).toBe(true); + }); + + it("should be ineligible when env var not in skill .env or process.env", () => { + delete process.env.MISSING_KEY; + + const skill = createSkill("test", { + name: "Test Skill", + metadata: { + requires: { + env: ["MISSING_KEY"], + }, + }, + }, "bundled", {}); + + const result = checkEligibility(skill, ctx("darwin")); + expect(result.eligible).toBe(false); + }); + it("should be eligible even if env var is empty string", () => { process.env.EMPTY_VAR = ""; diff --git a/packages/core/src/agent/skills/eligibility.ts b/packages/core/src/agent/skills/eligibility.ts index 3e1025fc..a89b44a9 100644 --- a/packages/core/src/agent/skills/eligibility.ts +++ b/packages/core/src/agent/skills/eligibility.ts @@ -19,7 +19,7 @@ import { normalizeRequirements, normalizePlatforms, } from "./types.js"; -import { credentialManager, getSkillsEnvPath } from "../credentials.js"; +import { dirname, join } from "node:path"; // ============================================================================ // Diagnostic Types @@ -74,11 +74,17 @@ export function binaryExists(binary: string): boolean { /** * Check if an environment variable is set * + * Checks the skill's own .env first, then falls back to process.env. + * * @param envVar - Environment variable name + * @param skill - Skill to check against * @returns True if set (even if empty string) */ -function envExists(envVar: string): boolean { - return credentialManager.hasEnv(envVar); +function envExists(envVar: string, skill: Skill): boolean { + if (Object.prototype.hasOwnProperty.call(skill.env, envVar)) { + return true; + } + return envVar in process.env; } // ============================================================================ @@ -306,7 +312,7 @@ export function checkEligibilityDetailed( if (requirements.env && requirements.env.length > 0) { for (const envVar of requirements.env) { // Check if env var exists - if (envExists(envVar)) continue; + if (envExists(envVar, skill)) continue; missingEnvVars.push(envVar); reasons.push(`Required environment variable not set: ${envVar}`); @@ -437,14 +443,14 @@ function getBinaryInstallHint(binary: string, platform: NodeJS.Platform): string /** * Generate hints for missing environment variables */ -function generateEnvHint(envVars: string[], _skill: Skill): string { +function generateEnvHint(envVars: string[], skill: Skill): string { const hints: string[] = []; + const envPath = join(dirname(skill.filePath), ".env"); for (const envVar of envVars) { // Check for well-known API key patterns if (envVar.endsWith("_API_KEY") || envVar.endsWith("_KEY")) { - const service = envVar.replace(/_API_KEY$|_KEY$/, "").toLowerCase(); - hints.push(`Set ${envVar} in your environment or add to ${getSkillsEnvPath()}`); + hints.push(`Set ${envVar} in ${envPath} or your environment`); // Add provider-specific hints const providerHint = getApiKeyHint(envVar); @@ -452,7 +458,7 @@ function generateEnvHint(envVars: string[], _skill: Skill): string { hints.push(providerHint); } } else { - hints.push(`export ${envVar}=`); + hints.push(`Set ${envVar} in ${envPath} or export ${envVar}=`); } } diff --git a/packages/core/src/agent/skills/index.ts b/packages/core/src/agent/skills/index.ts index 73104f48..72c58962 100644 --- a/packages/core/src/agent/skills/index.ts +++ b/packages/core/src/agent/skills/index.ts @@ -72,6 +72,7 @@ export { } from "./eligibility.js"; export { parseFrontmatter, parseSkillFile } from "./parser.js"; +export { parseDotEnv } from "./dotenv.js"; export { loadAllSkills, getProfileSkillsDir, initializeManagedSkills, getManagedSkillsDir } from "./loader.js"; // Export install module diff --git a/packages/core/src/agent/skills/loader.ts b/packages/core/src/agent/skills/loader.ts index b193c0aa..fed8006d 100644 --- a/packages/core/src/agent/skills/loader.ts +++ b/packages/core/src/agent/skills/loader.ts @@ -198,8 +198,8 @@ export function initializeManagedSkills(): void { // Check if skill exists in managed if (!existsSync(dest)) { - // Skill doesn't exist, copy it - cpSync(src, dest, { recursive: true, dereference: true }); + // Skill doesn't exist, copy it (skip .env files - those are user-specific) + cpSync(src, dest, { recursive: true, dereference: true, filter: (s) => !s.endsWith("/.env") }); continue; } @@ -214,9 +214,21 @@ export function initializeManagedSkills(): void { // Update if bundled version is higher if (compareVersions(bundledVersion, managedVersion) > 0) { - // Remove old and copy new + // Preserve user's .env file across upgrades + const envPath = join(dest, ".env"); + let savedEnv: string | null = null; + if (existsSync(envPath)) { + try { savedEnv = readFileSync(envPath, "utf-8"); } catch { /* ignore */ } + } + + // Remove old and copy new (skip .env from bundle) rmSync(dest, { recursive: true }); - cpSync(src, dest, { recursive: true, dereference: true }); + cpSync(src, dest, { recursive: true, dereference: true, filter: (s) => !s.endsWith("/.env") }); + + // Restore user's .env + if (savedEnv !== null) { + writeFileSync(envPath, savedEnv, "utf-8"); + } } } diff --git a/packages/core/src/agent/skills/parser.test.ts b/packages/core/src/agent/skills/parser.test.ts index ff21f99c..d250edb5 100644 --- a/packages/core/src/agent/skills/parser.test.ts +++ b/packages/core/src/agent/skills/parser.test.ts @@ -1,5 +1,8 @@ -import { describe, it, expect } from "vitest"; -import { parseFrontmatter } from "./parser.js"; +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { parseFrontmatter, parseSkillFile } from "./parser.js"; describe("parser", () => { describe("parseFrontmatter", () => { @@ -240,4 +243,126 @@ Body. expect(body).toBe("Body."); }); }); + + describe("parseSkillFile", () => { + const testDir = join(tmpdir(), `multica-parser-test-${Date.now()}`); + + beforeEach(() => { + mkdirSync(testDir, { recursive: true }); + }); + + afterEach(() => { + rmSync(testDir, { recursive: true, force: true }); + }); + + it("should parse metadata.requires fields", () => { + const skillDir = join(testDir, "test-skill"); + mkdirSync(skillDir); + writeFileSync(join(skillDir, "SKILL.md"), `--- +name: Test Skill +metadata: + requires: + env: + - API_KEY + - SECRET + bins: + - node + anyBins: + - whisper + - whisper-cli + config: + - browser.enabled +--- +Instructions. +`); + + const skill = parseSkillFile(join(skillDir, "SKILL.md"), "test-skill", "bundled"); + expect(skill).not.toBeNull(); + expect(skill!.frontmatter.metadata?.requires).toEqual({ + env: ["API_KEY", "SECRET"], + bins: ["node"], + anyBins: ["whisper", "whisper-cli"], + config: ["browser.enabled"], + }); + }); + + it("should parse metadata.always flag", () => { + const skillDir = join(testDir, "always-skill"); + mkdirSync(skillDir); + writeFileSync(join(skillDir, "SKILL.md"), `--- +name: Always Skill +metadata: + always: true +--- +Instructions. +`); + + const skill = parseSkillFile(join(skillDir, "SKILL.md"), "always-skill", "bundled"); + expect(skill).not.toBeNull(); + expect(skill!.frontmatter.metadata?.always).toBe(true); + }); + + it("should parse metadata.os field", () => { + const skillDir = join(testDir, "os-skill"); + mkdirSync(skillDir); + writeFileSync(join(skillDir, "SKILL.md"), `--- +name: OS Skill +metadata: + os: + - darwin + - linux +--- +Instructions. +`); + + const skill = parseSkillFile(join(skillDir, "SKILL.md"), "os-skill", "bundled"); + expect(skill).not.toBeNull(); + expect(skill!.frontmatter.metadata?.os).toEqual(["darwin", "linux"]); + }); + + it("should parse metadata.skillKey field", () => { + const skillDir = join(testDir, "key-skill"); + mkdirSync(skillDir); + writeFileSync(join(skillDir, "SKILL.md"), `--- +name: Key Skill +metadata: + skillKey: custom-key +--- +Instructions. +`); + + const skill = parseSkillFile(join(skillDir, "SKILL.md"), "key-skill", "bundled"); + expect(skill).not.toBeNull(); + expect(skill!.frontmatter.metadata?.skillKey).toBe("custom-key"); + }); + + it("should load .env file from skill directory", () => { + const skillDir = join(testDir, "env-skill"); + mkdirSync(skillDir); + writeFileSync(join(skillDir, "SKILL.md"), `--- +name: Env Skill +--- +Instructions. +`); + writeFileSync(join(skillDir, ".env"), "API_KEY=test-key\nSECRET=test-secret\n"); + + const skill = parseSkillFile(join(skillDir, "SKILL.md"), "env-skill", "bundled"); + expect(skill).not.toBeNull(); + expect(skill!.env).toEqual({ API_KEY: "test-key", SECRET: "test-secret" }); + }); + + it("should return empty env when no .env file exists", () => { + const skillDir = join(testDir, "no-env-skill"); + mkdirSync(skillDir); + writeFileSync(join(skillDir, "SKILL.md"), `--- +name: No Env Skill +--- +Instructions. +`); + + const skill = parseSkillFile(join(skillDir, "SKILL.md"), "no-env-skill", "bundled"); + expect(skill).not.toBeNull(); + expect(skill!.env).toEqual({}); + }); + }); }); diff --git a/packages/core/src/agent/skills/parser.ts b/packages/core/src/agent/skills/parser.ts index 78d4b57d..6c2d2d1a 100644 --- a/packages/core/src/agent/skills/parser.ts +++ b/packages/core/src/agent/skills/parser.ts @@ -4,9 +4,11 @@ * Parse skill files with YAML frontmatter and markdown body */ -import { readFileSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; import { parse as parseYaml } from "yaml"; -import type { Skill, SkillFrontmatter, SkillSource } from "./types.js"; +import type { Skill, SkillFrontmatter, SkillSource, SkillInstallSpec } from "./types.js"; +import { parseDotEnv } from "./dotenv.js"; /** * Parse YAML frontmatter from markdown content @@ -72,21 +74,37 @@ function validateFrontmatter(raw: Record): SkillFrontmatter | n // Parse metadata if present if (typeof raw.metadata === "object" && raw.metadata !== null) { const meta = raw.metadata as Record; + const filterStrings = (arr: unknown): string[] | undefined => + Array.isArray(arr) ? arr.filter((v): v is string => typeof v === "string") : undefined; + frontmatter.metadata = { emoji: typeof meta.emoji === "string" ? meta.emoji : undefined, - requiresEnv: Array.isArray(meta.requiresEnv) - ? meta.requiresEnv.filter((v): v is string => typeof v === "string") - : undefined, - requiresBinaries: Array.isArray(meta.requiresBinaries) - ? meta.requiresBinaries.filter((v): v is string => typeof v === "string") - : undefined, - platforms: Array.isArray(meta.platforms) - ? meta.platforms.filter((v): v is string => typeof v === "string") - : undefined, - tags: Array.isArray(meta.tags) - ? meta.tags.filter((v): v is string => typeof v === "string") - : undefined, + tags: filterStrings(meta.tags), + // Legacy fields + requiresEnv: filterStrings(meta.requiresEnv), + requiresBinaries: filterStrings(meta.requiresBinaries), + platforms: filterStrings(meta.platforms), + // New fields + always: typeof meta.always === "boolean" ? meta.always : undefined, + skillKey: typeof meta.skillKey === "string" ? meta.skillKey : undefined, + os: filterStrings(meta.os), }; + + // Parse requires nested object + if (typeof meta.requires === "object" && meta.requires !== null) { + const req = meta.requires as Record; + frontmatter.metadata.requires = { + bins: filterStrings(req.bins), + anyBins: filterStrings(req.anyBins), + env: filterStrings(req.env), + config: filterStrings(req.config), + }; + } + + // Parse install array + if (Array.isArray(meta.install)) { + frontmatter.metadata.install = meta.install as SkillInstallSpec[]; + } } // Parse invocation control fields @@ -170,12 +188,25 @@ export function parseSkillFile( return null; } + // Load .env from skill directory + const skillDir = dirname(filePath); + const envPath = join(skillDir, ".env"); + let env: Record = {}; + if (existsSync(envPath)) { + try { + env = parseDotEnv(readFileSync(envPath, "utf-8")); + } catch { + // Ignore .env parse errors + } + } + return { id: skillId, frontmatter, instructions, source, filePath, + env, }; } catch { // File read error or other issues diff --git a/packages/core/src/agent/skills/types.ts b/packages/core/src/agent/skills/types.ts index 0369a491..abb440bd 100644 --- a/packages/core/src/agent/skills/types.ts +++ b/packages/core/src/agent/skills/types.ts @@ -147,6 +147,8 @@ export interface Skill { source: SkillSource; /** Full path to SKILL.md */ filePath: string; + /** Environment variables loaded from the skill's .env file */ + env: Record; } // ============================================================================