feat(skills): validate manifest permissions
This commit is contained in:
@@ -10,6 +10,7 @@ import type { AuditEvent } from '../audit/types.js';
|
|||||||
import type { Config } from '../config/schema.js';
|
import type { Config } from '../config/schema.js';
|
||||||
import type { Skill } from '../skills/index.js';
|
import type { Skill } from '../skills/index.js';
|
||||||
import { loadAllSkills, SkillInstaller, buildInstallerPlan, loadSkill } from '../skills/index.js';
|
import { loadAllSkills, SkillInstaller, buildInstallerPlan, loadSkill } from '../skills/index.js';
|
||||||
|
import type { SkillPermissions } from '../skills/types.js';
|
||||||
import { loadConfigSafe } from './shared.js';
|
import { loadConfigSafe } from './shared.js';
|
||||||
|
|
||||||
export interface SkillListRow {
|
export interface SkillListRow {
|
||||||
@@ -31,6 +32,7 @@ export interface SkillInstallerPlanView {
|
|||||||
name: string;
|
name: string;
|
||||||
tier: Skill['manifest']['tier'];
|
tier: Skill['manifest']['tier'];
|
||||||
version: string;
|
version: string;
|
||||||
|
permissions?: SkillPermissions;
|
||||||
};
|
};
|
||||||
mode: 'dry-run';
|
mode: 'dry-run';
|
||||||
steps: Array<{ installerType: string; command: string }>;
|
steps: Array<{ installerType: string; command: string }>;
|
||||||
@@ -45,6 +47,39 @@ export interface SkillInstallPreflightView {
|
|||||||
skipped: SkillInstallerPlanView['skipped'];
|
skipped: SkillInstallerPlanView['skipped'];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function renderSkillPermissions(perms?: SkillPermissions): string[] {
|
||||||
|
if (!perms) {
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
|
||||||
|
const lines: string[] = ['Permissions:'];
|
||||||
|
if (perms.tool_groups && perms.tool_groups.length > 0) {
|
||||||
|
lines.push(`- tool_groups: ${perms.tool_groups.join(', ')}`);
|
||||||
|
}
|
||||||
|
if (perms.tools && perms.tools.length > 0) {
|
||||||
|
lines.push(`- tools: ${perms.tools.join(', ')}`);
|
||||||
|
}
|
||||||
|
if (perms.fs?.read && perms.fs.read.length > 0) {
|
||||||
|
lines.push(`- fs.read: ${perms.fs.read.join(', ')}`);
|
||||||
|
}
|
||||||
|
if (perms.fs?.write && perms.fs.write.length > 0) {
|
||||||
|
lines.push(`- fs.write: ${perms.fs.write.join(', ')}`);
|
||||||
|
}
|
||||||
|
if (perms.net && perms.net.length > 0) {
|
||||||
|
lines.push(`- net: ${perms.net.map((n) => `${n.host}${n.ports && n.ports.length > 0 ? `:${n.ports.join('|')}` : ''}`).join(', ')}`);
|
||||||
|
}
|
||||||
|
if (perms.secrets && perms.secrets.length > 0) {
|
||||||
|
lines.push(`- secrets: ${perms.secrets.join(', ')}`);
|
||||||
|
}
|
||||||
|
if (perms.execution_environment) {
|
||||||
|
lines.push(`- execution_environment: ${perms.execution_environment}`);
|
||||||
|
}
|
||||||
|
if (lines.length === 1) {
|
||||||
|
return ['Permissions: (none declared)'];
|
||||||
|
}
|
||||||
|
return lines;
|
||||||
|
}
|
||||||
|
|
||||||
export interface SkillInstallerExecutionStubView {
|
export interface SkillInstallerExecutionStubView {
|
||||||
skill: SkillInstallerPlanView['skill'];
|
skill: SkillInstallerPlanView['skill'];
|
||||||
execution: 'stub';
|
execution: 'stub';
|
||||||
@@ -667,6 +702,10 @@ export function renderSkillInfo(skill: Skill): string {
|
|||||||
lines.push(`Tools: ${skill.manifest.tools.join(', ')}`);
|
lines.push(`Tools: ${skill.manifest.tools.join(', ')}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (skill.manifest.permissions) {
|
||||||
|
lines.push(...renderSkillPermissions(skill.manifest.permissions));
|
||||||
|
}
|
||||||
|
|
||||||
if (skill.unavailableReasons && skill.unavailableReasons.length > 0) {
|
if (skill.unavailableReasons && skill.unavailableReasons.length > 0) {
|
||||||
lines.push(`Unavailable reasons: ${skill.unavailableReasons.join('; ')}`);
|
lines.push(`Unavailable reasons: ${skill.unavailableReasons.join('; ')}`);
|
||||||
}
|
}
|
||||||
@@ -698,6 +737,7 @@ export function toSkillInstallerPlanView(skill: Skill): SkillInstallerPlanView {
|
|||||||
name: skill.manifest.name,
|
name: skill.manifest.name,
|
||||||
tier: skill.manifest.tier,
|
tier: skill.manifest.tier,
|
||||||
version: skill.manifest.version,
|
version: skill.manifest.version,
|
||||||
|
permissions: skill.manifest.permissions,
|
||||||
},
|
},
|
||||||
mode: plan.mode,
|
mode: plan.mode,
|
||||||
steps: plan.steps,
|
steps: plan.steps,
|
||||||
@@ -711,6 +751,10 @@ export function renderSkillInstallerPlan(view: SkillInstallerPlanView): string {
|
|||||||
`Mode: ${view.mode}`,
|
`Mode: ${view.mode}`,
|
||||||
];
|
];
|
||||||
|
|
||||||
|
if (view.skill.permissions) {
|
||||||
|
lines.push(...renderSkillPermissions(view.skill.permissions));
|
||||||
|
}
|
||||||
|
|
||||||
if (view.steps.length === 0) {
|
if (view.steps.length === 0) {
|
||||||
lines.push('Planned steps: none');
|
lines.push('Planned steps: none');
|
||||||
} else {
|
} else {
|
||||||
@@ -754,6 +798,10 @@ export function renderSkillInstallPreflight(view: SkillInstallPreflightView): st
|
|||||||
`Mode: ${view.mode}`,
|
`Mode: ${view.mode}`,
|
||||||
];
|
];
|
||||||
|
|
||||||
|
if (view.skill.permissions) {
|
||||||
|
lines.push(...renderSkillPermissions(view.skill.permissions));
|
||||||
|
}
|
||||||
|
|
||||||
if (view.steps.length === 0) {
|
if (view.steps.length === 0) {
|
||||||
lines.push('Planned installer steps: none');
|
lines.push('Planned installer steps: none');
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -203,6 +203,28 @@ describe('loadSkill', () => {
|
|||||||
expect(skill).toBeNull();
|
expect(skill).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('returns null when manifest.json has invalid permissions specification', () => {
|
||||||
|
tmpDir = mkdtempSync(join(tmpdir(), 'flynn-test-'));
|
||||||
|
const skillDir = join(tmpDir, 'invalid-permissions');
|
||||||
|
mkdirSync(skillDir);
|
||||||
|
writeFileSync(
|
||||||
|
join(skillDir, 'manifest.json'),
|
||||||
|
JSON.stringify({
|
||||||
|
name: 'invalid-permissions',
|
||||||
|
description: 'Bad permissions',
|
||||||
|
version: '1.0.0',
|
||||||
|
permissions: {
|
||||||
|
fs: { read: 'not-an-array' },
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
writeFileSync(join(skillDir, 'SKILL.md'), '# Invalid Permissions');
|
||||||
|
|
||||||
|
const skill = loadSkill(skillDir, 'bundled');
|
||||||
|
|
||||||
|
expect(skill).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
it('strips markdown heading markers from inferred description', () => {
|
it('strips markdown heading markers from inferred description', () => {
|
||||||
// Positive: a first line like "## My Heading" should become "My Heading".
|
// Positive: a first line like "## My Heading" should become "My Heading".
|
||||||
tmpDir = mkdtempSync(join(tmpdir(), 'flynn-test-'));
|
tmpDir = mkdtempSync(join(tmpdir(), 'flynn-test-'));
|
||||||
|
|||||||
@@ -53,6 +53,84 @@ function hasValidInstallers(manifest: unknown): boolean {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isNumberArray(value: unknown): value is number[] {
|
||||||
|
return Array.isArray(value) && value.every((item) => typeof item === 'number' && Number.isFinite(item));
|
||||||
|
}
|
||||||
|
|
||||||
|
function hasValidPermissions(manifest: unknown): boolean {
|
||||||
|
if (!manifest || typeof manifest !== 'object') {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
const candidate = manifest as { permissions?: unknown };
|
||||||
|
if (candidate.permissions === undefined) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!candidate.permissions || typeof candidate.permissions !== 'object') {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
const perms = candidate.permissions as {
|
||||||
|
tool_groups?: unknown;
|
||||||
|
tools?: unknown;
|
||||||
|
fs?: unknown;
|
||||||
|
net?: unknown;
|
||||||
|
secrets?: unknown;
|
||||||
|
execution_environment?: unknown;
|
||||||
|
};
|
||||||
|
|
||||||
|
if (perms.tool_groups !== undefined && !isStringArray(perms.tool_groups)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if (perms.tools !== undefined && !isStringArray(perms.tools)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (perms.fs !== undefined) {
|
||||||
|
if (!perms.fs || typeof perms.fs !== 'object') {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const fsPerms = perms.fs as { read?: unknown; write?: unknown };
|
||||||
|
if (fsPerms.read !== undefined && !isStringArray(fsPerms.read)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if (fsPerms.write !== undefined && !isStringArray(fsPerms.write)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (perms.net !== undefined) {
|
||||||
|
if (!Array.isArray(perms.net)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
for (const entry of perms.net) {
|
||||||
|
if (!entry || typeof entry !== 'object') {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const e = entry as { host?: unknown; ports?: unknown };
|
||||||
|
if (typeof e.host !== 'string' || e.host.trim().length === 0) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if (e.ports !== undefined && !isNumberArray(e.ports)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (perms.secrets !== undefined && !isStringArray(perms.secrets)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (perms.execution_environment !== undefined) {
|
||||||
|
if (perms.execution_environment !== 'sandbox' && perms.execution_environment !== 'host') {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Check whether a skill's system requirements are met.
|
* Check whether a skill's system requirements are met.
|
||||||
*
|
*
|
||||||
@@ -136,6 +214,10 @@ export function loadSkill(directory: string, tier: SkillTier): Skill | null {
|
|||||||
console.warn(`Skill manifest at ${manifestPath} has invalid installers specification`);
|
console.warn(`Skill manifest at ${manifestPath} has invalid installers specification`);
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
if (!hasValidPermissions(raw)) {
|
||||||
|
console.warn(`Skill manifest at ${manifestPath} has invalid permissions specification`);
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
manifest = {
|
manifest = {
|
||||||
...raw,
|
...raw,
|
||||||
|
|||||||
@@ -50,6 +50,39 @@ export type SkillInstallerSpec =
|
|||||||
| GoInstallerSpec
|
| GoInstallerSpec
|
||||||
| DownloadInstallerSpec;
|
| DownloadInstallerSpec;
|
||||||
|
|
||||||
|
export interface SkillFsPermissions {
|
||||||
|
/** Allowed path globs for read-only filesystem access. */
|
||||||
|
read?: string[];
|
||||||
|
/** Allowed path globs for write filesystem access (includes edits/patches). */
|
||||||
|
write?: string[];
|
||||||
|
}
|
||||||
|
|
||||||
|
export interface SkillNetPermission {
|
||||||
|
/** Host glob, e.g. "api.todoist.com" or "*.example.com". */
|
||||||
|
host: string;
|
||||||
|
/** Allowed ports. If omitted, any port is allowed. */
|
||||||
|
ports?: number[];
|
||||||
|
}
|
||||||
|
|
||||||
|
export interface SkillPermissions {
|
||||||
|
/** Allowed tool groups, e.g. ["group:fs", "group:web"]. */
|
||||||
|
tool_groups?: string[];
|
||||||
|
/** Allowed tool name patterns (glob). Overrides tool_groups when present. */
|
||||||
|
tools?: string[];
|
||||||
|
/** Filesystem access constraints (optional). */
|
||||||
|
fs?: SkillFsPermissions;
|
||||||
|
/** Network access constraints (optional). */
|
||||||
|
net?: SkillNetPermission[];
|
||||||
|
/** Named secret scopes required for credentialed actions (optional). */
|
||||||
|
secrets?: string[];
|
||||||
|
/**
|
||||||
|
* Execution environment preference for high-risk operations.
|
||||||
|
* - sandbox: run high-risk tools in the session sandbox (default)
|
||||||
|
* - host: allow high-risk tools on the host (escape hatch)
|
||||||
|
*/
|
||||||
|
execution_environment?: 'sandbox' | 'host';
|
||||||
|
}
|
||||||
|
|
||||||
/** Manifest for a skill (manifest.json). */
|
/** Manifest for a skill (manifest.json). */
|
||||||
export interface SkillManifest {
|
export interface SkillManifest {
|
||||||
/** Unique skill name (e.g., 'git', 'web-search'). */
|
/** Unique skill name (e.g., 'git', 'web-search'). */
|
||||||
@@ -70,6 +103,8 @@ export interface SkillManifest {
|
|||||||
dependencies?: string[];
|
dependencies?: string[];
|
||||||
/** Optional dependency installers for future automated setup. */
|
/** Optional dependency installers for future automated setup. */
|
||||||
installers?: SkillInstallerSpec[];
|
installers?: SkillInstallerSpec[];
|
||||||
|
/** Optional capability declarations. Used for runtime policy enforcement. */
|
||||||
|
permissions?: SkillPermissions;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** A loaded skill ready for use. */
|
/** A loaded skill ready for use. */
|
||||||
|
|||||||
Reference in New Issue
Block a user