mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-02 12:22:22 +00:00
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
Publish `librechat-data-provider` to NPM / pack (push) Has been cancelled
Publish `librechat-data-provider` to NPM / publish-npm (push) Has been cancelled
* feat: Add GitHub skill sync
* fix: Address GitHub skill sync CI
* fix: Harden GitHub skill sync review paths
* fix: Prevent overlapping skill sync runs
* fix: Address GitHub skill sync review findings
* fix: Satisfy Git ref lint rule
* fix: Address GitHub sync review follow-ups
* fix: Match skill frontmatter closing fence
* fix: Address GitHub sync review cycle
* fix: Address GitHub sync review follow-ups
* fix: Harden GitHub skill sync worker
* fix: Format GitHub sync rollback log
* fix: Address GitHub sync review feedback
* fix: Format skill import parse handling
* fix: Coerce scalar skill frontmatter and correct scheduler timer clear
- parse: coerce numeric/boolean name and description scalars to strings instead of dropping them to empty (restores pre-refactor behavior; preserves absent-vs-empty distinction for the when-to-use fallback)
- scheduler: clear the setTimeout handle with clearTimeout rather than clearInterval
- test: cover non-string scalar frontmatter coercion
* fix: Tolerate trailing whitespace after SKILL.md opening frontmatter fence
extractFrontmatterBlock required the opening fence to be exactly '---\n', so an opener with trailing spaces/tabs (e.g. '--- \n') silently dropped all frontmatter even though the closing-fence regex already tolerates it. Match the opener with /^---[ \t]*\n/ for symmetry. Addresses Codex P3 (parse.ts:24).
* feat: Run GitHub skill sync under a per-source tenant context
Under TENANT_ISOLATION_STRICT, the sync ran with no async tenant context, so the tenant-isolation mongoose hooks threw on every Skill/SkillFile/AclEntry operation; in non-strict mode synced skills were written tenant-less and never matched tenant-scoped reads. Add an optional per-source tenantId to the skillSync config; when set, each source sync runs inside tenantStorage.run({ tenantId }) so skills, files, and public ACL grants are created and listed within that tenant, and the skill row is stamped with the tenantId for correct dedup. Sources without tenantId keep the prior single-tenant behavior. Avoids runAsSystem. Addresses Codex P2 (sync.js:70).
Lock/status/credential bookkeeping stays outside the tenant context (those collections are intentionally global).
* test: Restore dropped tenant-context coverage for GitHub skill sync
The prior commit shipped the getTenantId import in github.spec.ts without the tenant tests that use it (lost in an interrupted edit), which failed the eslint --max-warnings=0 CI job on an unused import. Restore both github.spec.ts tenant tests (tenant-scoped run stamps tenantId and executes inside the tenant ALS context; no-tenant run stays ambient) and the two config-schemas tenant tests (accepts tenantId, rejects __SYSTEM__).
* test: Restore dropped github.spec tenant-context tests
The previous commit's github.spec.ts edit did not apply (anchor mismatch), so the getTenantId import remained unused and failed eslint --max-warnings=0. Add the two tenant tests that use it: a tenant-scoped run stamps tenantId and executes inside the tenant ALS context, and a no-tenant run stays ambient.
* feat: Scope synced skill author to tenant and harden tenant-context sync
Addresses the latest Codex review on the per-source tenant change:
- makeSourceAuthorId now folds tenantId into the synthetic author hash so the
same source mirrored into different tenants gets distinct author ids (clearer
audits, no cross-tenant author collisions). Single-tenant author ids stay
stable (suffix omitted when tenantId is absent).
- syncSourceInTenantContext uses an async callback per the tenant-context
contract so the ALS store propagates across awaited Mongoose calls.
- Tests: same-source/different-tenant yields distinct authors; mirror cleanup
is scoped to the source and deletes only its absent-upstream skills.
* fix: Repair tsc error and guard external edits in github skill sync
- Fix TS2352 in github.spec mirror-cleanup test: build the existing-skill mock via makeSkill with authorName instead of an under-typed 'as CreateSkillInput' cast (this was the failing TypeScript CI check on f00ce3c5a).
- 808: commitExistingRemoteSkillAfterFileSync re-reads to clear our own file-sync version bumps, but now compares refreshed content against the pre-sync snapshot (body/name/description/always-apply) and throws SKILL_CONFLICT on a concurrent external edit instead of overwriting it.
* docs: Note skillSync source tenantId is effectively immutable
Changing/adding/removing a source's tenantId orphans previously mirrored skills in the old tenant (a tenant-scoped sync cannot clean another tenant's data without runAsSystem, which is intentionally avoided).
* fix: Key GitHub skill upstream identity on source id and path only
Addresses Codex finding (github.ts:217): makeUpstreamId previously included owner/repo, so repointing a source to a renamed or replacement repository (same source id) changed the upstreamId, made findSkillBySourceIdentity miss the existing mirror, and then collided on the (name, author, tenantId) uniqueness constraint — leaving the source stuck failing. Identity now keys on the stable source id + root path only. The feature is unreleased, so there is no stored-id migration. Updated spec upstreamId fixtures to the new format; the existing ref-independent identity test now also covers repo moves.
* fix: Scope GitHub skill mirror deletion to the source tenant
Addresses Codex P1 (github.ts:1047/1057): an ambient source (no tenantId) runs listSkillsBySource without tenant context, which under non-strict isolation returns github-synced skills across all tenants. The mirror-deletion pass then treated other tenants' skills as absent-upstream and could delete them. Filter existingSyncedSkills to rows whose tenantId matches the source's configured tenantId (absent = its own ambient bucket) before deleting, so a sync never removes another tenant's mirrored skills. Covered by a test where an ambient run leaves a tenant-b-owned skill untouched.
* fix: Apply tenant-scoped mirror deletion implementation
The prior commit (75ccfa3fc) added the test but the source change to github.ts was lost in an interrupted edit, leaving a failing test with no implementation. This adds the actual guard: the mirror-deletion pass skips skills whose tenantId does not match the source's configured tenantId (absent = ambient bucket), so an ambient source whose listSkillsBySource returns cross-tenant rows under non-strict isolation cannot delete another tenant's mirrored skills.
* fix: Resolve global access role outside tenant context for synced skill grants
Addresses Codex P2 (github.ts:1166): default access roles (incl. skill_viewer) are seeded globally with no tenantId under runAsSystem, but a tenant-scoped sync wraps ensurePublicViewer in the source's tenant context. The PermissionService grantPermission resolved the role via a tenant-isolated AccessRole query, so the global role did not match and tenant-scoped syncs failed with 'Role skill_viewer not found'. The sync adapter now resolves the role inside runAsSystem (matching the global seed) and writes the ACL entry in the active tenant context, so the AclEntry is tenant-scoped (visible to tenant users) while the role lookup still succeeds. Covered by service tests for the resolve-vs-write split and the missing-role failure.
* fix: Strip placeholder frontmatter booleans and check skill conflict before file sync
- 1083 (github.ts:759): toCleanFrontmatter now drops a non-boolean always-apply (e.g. the 'always-apply:' / 'always-apply: # TODO' placeholder, which js-yaml yields as null). The boolean is already captured in the dedicated alwaysApply field; persisting null left ambiguous frontmatter on the synced skill.
- 1080 (github.ts:1057): for an existing mirrored skill, check for an external content edit (via getSkillById + hasExternalSkillEdit) BEFORE syncSkillFiles mutates the bundled files, so a concurrently edited skill fails fast with SKILL_CONFLICT without partial file rewrites. The post-file-sync check still guards edits that land during the file sync window.
Tests: placeholder always-apply is dropped from synced frontmatter; concurrent-edit conflict leaves files unmutated (no upsert/delete).
* fix: Harden GitHub skill sync review paths
* fix: Reuse moved GitHub skill mirrors
* fix: Scope GitHub sync identity conflicts
* test: Fix GitHub sync conflict mock typing
* fix: Support nested env-backed skill sync
* fix: Keep skill sync config base-only
* fix: Scope GitHub skill identity lookup by tenant
* fix: Harden GitHub skill sync admin gates
* fix: Guard existing skill sync permission grants
* feat: Trigger skill sync from resolved config
* fix: Scope resolved skill sync by tenant
* test: Allow manual skill sync status tenant scoping
* refactor: Extract skill sync trigger orchestrator
* test: Complete orchestrator status fixture
* chore: Bump data provider version
* fix: Restrict skill sync server credentials
* test: Complete admin skill sync status fixtures
* fix: tighten skill sync trigger safeguards
* fix: preserve alwaysApply skill sync alias
* chore: sort skill sync imports
* fix: preserve skill sync request scope
* fix: harden skill sync review edges
* refactor: move skill sync admin access to api package
* fix: add skill sync declaration return types
* fix: satisfy skill sync type checks
* fix: resolve codex skill sync review findings
* fix: harden skill sync review edges
* fix: resolve codex skill sync edge findings
* fix: satisfy API declaration build after rebase
628 lines
20 KiB
JavaScript
628 lines
20 KiB
JavaScript
const express = require('express');
|
|
const request = require('supertest');
|
|
const JSZip = require('jszip');
|
|
const mongoose = require('mongoose');
|
|
const { MongoMemoryServer } = require('mongodb-memory-server');
|
|
|
|
jest.mock('librechat-data-provider', () => {
|
|
const actual = jest.requireActual('librechat-data-provider');
|
|
return {
|
|
...actual,
|
|
mergeFileConfig: jest.fn((dynamic) => {
|
|
const skillFileSizeLimit = dynamic?.skills?.fileSizeLimit;
|
|
return {
|
|
...actual.fileConfig,
|
|
...dynamic,
|
|
skills: {
|
|
...(actual.fileConfig.skills ?? { fileSizeLimit: 50 * 1024 * 1024 }),
|
|
...(skillFileSizeLimit !== undefined
|
|
? { fileSizeLimit: skillFileSizeLimit * 1024 * 1024 }
|
|
: {}),
|
|
},
|
|
};
|
|
}),
|
|
};
|
|
});
|
|
|
|
const {
|
|
SystemRoles,
|
|
ResourceType,
|
|
AccessRoleIds,
|
|
PrincipalType,
|
|
PermissionBits,
|
|
} = require('librechat-data-provider');
|
|
|
|
let mockFileConfig;
|
|
const mockMaybeRunGitHubSkillSyncForRequest = jest.fn(async () => false);
|
|
|
|
jest.mock('~/server/services/Config', () => ({
|
|
getCachedTools: jest.fn().mockResolvedValue({}),
|
|
getAppConfig: jest.fn().mockResolvedValue({
|
|
fileStrategy: 'local',
|
|
paths: { uploads: '/tmp/uploads', images: '/tmp/images' },
|
|
}),
|
|
}));
|
|
|
|
jest.mock('~/server/middleware/config/app', () => (req, _res, next) => {
|
|
req.config = {
|
|
fileStrategy: 'local',
|
|
paths: { uploads: '/tmp/uploads', images: '/tmp/images' },
|
|
fileConfig: mockFileConfig,
|
|
};
|
|
next();
|
|
});
|
|
|
|
jest.mock('~/server/services/Files/strategies', () => ({
|
|
getStrategyFunctions: jest.fn().mockReturnValue({
|
|
saveBuffer: jest.fn().mockResolvedValue('/uploads/test/file.txt'),
|
|
getDownloadStream: jest.fn().mockResolvedValue({
|
|
pipe: jest.fn(),
|
|
on: jest.fn(),
|
|
[Symbol.asyncIterator]: async function* () {
|
|
yield Buffer.from('test content');
|
|
},
|
|
}),
|
|
}),
|
|
}));
|
|
|
|
jest.mock('~/server/utils/getFileStrategy', () => ({
|
|
getFileStrategy: jest.fn().mockReturnValue('local'),
|
|
}));
|
|
|
|
jest.mock('~/server/services/Skills/sync', () => ({
|
|
maybeRunGitHubSkillSyncForRequest: mockMaybeRunGitHubSkillSyncForRequest,
|
|
}));
|
|
|
|
jest.mock('~/models', () => {
|
|
const mongoose = require('mongoose');
|
|
const { createMethods } = require('@librechat/data-schemas');
|
|
const methods = createMethods(mongoose, {
|
|
removeAllPermissions: async ({ resourceType, resourceId }) => {
|
|
const AclEntry = mongoose.models.AclEntry;
|
|
if (AclEntry) {
|
|
await AclEntry.deleteMany({ resourceType, resourceId });
|
|
}
|
|
},
|
|
});
|
|
// Override getRoleByName to return a permissive SKILLS capability block for all
|
|
// test users. The real role seeding relies on `initializeRoles` which this
|
|
// suite intentionally skips to keep setup minimal.
|
|
return {
|
|
...methods,
|
|
getRoleByName: jest.fn(),
|
|
};
|
|
});
|
|
|
|
jest.mock('~/server/middleware', () => ({
|
|
requireJwtAuth: (req, res, next) => next(),
|
|
canAccessSkillResource: jest.requireActual('~/server/middleware').canAccessSkillResource,
|
|
}));
|
|
|
|
let app;
|
|
let mongoServer;
|
|
let skillRoutes;
|
|
let Skill;
|
|
let SkillFile;
|
|
let AclEntry;
|
|
let AccessRole;
|
|
let User;
|
|
let testUsers;
|
|
let testRoles;
|
|
let grantPermission;
|
|
let currentTestUser;
|
|
|
|
function setTestUser(user) {
|
|
currentTestUser = user;
|
|
}
|
|
|
|
beforeAll(async () => {
|
|
mongoServer = await MongoMemoryServer.create();
|
|
await mongoose.connect(mongoServer.getUri());
|
|
|
|
const dbModels = require('~/db/models');
|
|
Skill = dbModels.Skill;
|
|
SkillFile = dbModels.SkillFile;
|
|
AclEntry = dbModels.AclEntry;
|
|
AccessRole = dbModels.AccessRole;
|
|
User = dbModels.User;
|
|
|
|
const permissionService = require('~/server/services/PermissionService');
|
|
grantPermission = permissionService.grantPermission;
|
|
|
|
await setupTestData();
|
|
|
|
app = express();
|
|
app.use(express.json());
|
|
app.use((req, res, next) => {
|
|
if (currentTestUser) {
|
|
req.user = {
|
|
...(currentTestUser.toObject ? currentTestUser.toObject() : currentTestUser),
|
|
id: currentTestUser._id.toString(),
|
|
_id: currentTestUser._id,
|
|
name: currentTestUser.name,
|
|
role: currentTestUser.role,
|
|
};
|
|
}
|
|
next();
|
|
});
|
|
|
|
currentTestUser = testUsers.owner;
|
|
skillRoutes = require('./skills');
|
|
app.use('/api/skills', skillRoutes);
|
|
});
|
|
|
|
afterEach(async () => {
|
|
await Skill.deleteMany({});
|
|
await SkillFile.deleteMany({});
|
|
await AclEntry.deleteMany({});
|
|
currentTestUser = testUsers.owner;
|
|
mockFileConfig = undefined;
|
|
mockMaybeRunGitHubSkillSyncForRequest.mockClear();
|
|
});
|
|
|
|
afterAll(async () => {
|
|
await mongoose.disconnect();
|
|
await mongoServer.stop();
|
|
jest.clearAllMocks();
|
|
});
|
|
|
|
async function setupTestData() {
|
|
testRoles = {
|
|
viewer: await AccessRole.create({
|
|
accessRoleId: AccessRoleIds.SKILL_VIEWER,
|
|
name: 'Viewer',
|
|
resourceType: ResourceType.SKILL,
|
|
permBits: PermissionBits.VIEW,
|
|
}),
|
|
editor: await AccessRole.create({
|
|
accessRoleId: AccessRoleIds.SKILL_EDITOR,
|
|
name: 'Editor',
|
|
resourceType: ResourceType.SKILL,
|
|
permBits: PermissionBits.VIEW | PermissionBits.EDIT,
|
|
}),
|
|
owner: await AccessRole.create({
|
|
accessRoleId: AccessRoleIds.SKILL_OWNER,
|
|
name: 'Owner',
|
|
resourceType: ResourceType.SKILL,
|
|
permBits:
|
|
PermissionBits.VIEW | PermissionBits.EDIT | PermissionBits.DELETE | PermissionBits.SHARE,
|
|
}),
|
|
};
|
|
|
|
testUsers = {
|
|
owner: await User.create({
|
|
name: 'Skill Owner',
|
|
email: 'skill-owner@test.com',
|
|
role: SystemRoles.USER,
|
|
}),
|
|
editor: await User.create({
|
|
name: 'Skill Editor',
|
|
email: 'skill-editor@test.com',
|
|
role: SystemRoles.USER,
|
|
}),
|
|
noAccess: await User.create({
|
|
name: 'No Access',
|
|
email: 'no-access@test.com',
|
|
role: SystemRoles.USER,
|
|
}),
|
|
};
|
|
|
|
const { getRoleByName } = require('~/models');
|
|
getRoleByName.mockImplementation((roleName) => {
|
|
if (roleName === SystemRoles.USER || roleName === SystemRoles.ADMIN) {
|
|
return {
|
|
permissions: {
|
|
SKILLS: {
|
|
USE: true,
|
|
CREATE: true,
|
|
SHARE: true,
|
|
SHARE_PUBLIC: true,
|
|
},
|
|
},
|
|
};
|
|
}
|
|
return null;
|
|
});
|
|
}
|
|
|
|
async function createSkillAsOwner(overrides = {}) {
|
|
// Description is deliberately kept above the 20-char short-description
|
|
// warning threshold so existing tests don't trip the coaching warning.
|
|
const res = await request(app)
|
|
.post('/api/skills')
|
|
.send({
|
|
name: 'demo-skill',
|
|
description: 'A small demo skill used in routing integration tests.',
|
|
body: '# Demo',
|
|
...overrides,
|
|
});
|
|
return res;
|
|
}
|
|
|
|
describe('Skill routes', () => {
|
|
let errSpy;
|
|
beforeEach(() => {
|
|
errSpy = jest.spyOn(console, 'error').mockImplementation();
|
|
});
|
|
afterEach(() => errSpy.mockRestore());
|
|
|
|
describe('POST /api/skills', () => {
|
|
it('creates a skill and grants SKILL_OWNER ACL', async () => {
|
|
const res = await createSkillAsOwner();
|
|
expect(res.status).toBe(201);
|
|
expect(res.body._id).toBeDefined();
|
|
expect(res.body.version).toBe(1);
|
|
expect(res.body.name).toBe('demo-skill');
|
|
// No warnings on a description that comfortably clears the threshold.
|
|
expect(res.body.warnings).toBeUndefined();
|
|
|
|
const acl = await AclEntry.findOne({
|
|
resourceType: ResourceType.SKILL,
|
|
resourceId: res.body._id,
|
|
principalType: PrincipalType.USER,
|
|
principalId: testUsers.owner._id,
|
|
});
|
|
expect(acl).toBeTruthy();
|
|
expect(acl.roleId.toString()).toBe(testRoles.owner._id.toString());
|
|
});
|
|
|
|
it('attaches a TOO_SHORT warning on create when description is under 20 chars', async () => {
|
|
const res = await createSkillAsOwner({
|
|
name: 'short-desc-skill',
|
|
description: 'Too short.',
|
|
});
|
|
expect(res.status).toBe(201);
|
|
expect(res.body._id).toBeDefined();
|
|
expect(Array.isArray(res.body.warnings)).toBe(true);
|
|
expect(res.body.warnings).toEqual([
|
|
expect.objectContaining({
|
|
field: 'description',
|
|
code: 'TOO_SHORT',
|
|
severity: 'warning',
|
|
}),
|
|
]);
|
|
});
|
|
|
|
it('rejects names starting with reserved brand prefixes', async () => {
|
|
const anthropic = await createSkillAsOwner({ name: 'anthropic-helper' });
|
|
expect(anthropic.status).toBe(400);
|
|
const claude = await createSkillAsOwner({ name: 'claude-helper' });
|
|
expect(claude.status).toBe(400);
|
|
});
|
|
|
|
it('allows names that merely contain reserved brand words as substrings', async () => {
|
|
const res = await createSkillAsOwner({ name: 'research-anthropic-helper' });
|
|
expect(res.status).toBe(201);
|
|
});
|
|
|
|
it('rejects reserved CLI command names', async () => {
|
|
const res = await createSkillAsOwner({ name: 'settings' });
|
|
expect(res.status).toBe(400);
|
|
});
|
|
|
|
it('rejects frontmatter with unknown keys', async () => {
|
|
const res = await createSkillAsOwner({
|
|
name: 'bad-frontmatter-skill',
|
|
frontmatter: { 'not-a-real-key': 'value' },
|
|
});
|
|
expect(res.status).toBe(400);
|
|
expect(res.body.issues).toEqual(
|
|
expect.arrayContaining([expect.objectContaining({ code: 'UNKNOWN_KEY' })]),
|
|
);
|
|
});
|
|
|
|
it('rejects missing description with 400', async () => {
|
|
const res = await request(app).post('/api/skills').send({ name: 'x-skill', body: '' });
|
|
expect(res.status).toBe(400);
|
|
});
|
|
|
|
it('rejects invalid name with 400 validation failure', async () => {
|
|
const res = await createSkillAsOwner({ name: 'BAD NAME' });
|
|
expect(res.status).toBe(400);
|
|
expect(res.body.issues).toBeDefined();
|
|
});
|
|
|
|
it('rejects duplicate names with 409', async () => {
|
|
const a = await createSkillAsOwner();
|
|
expect(a.status).toBe(201);
|
|
const b = await createSkillAsOwner();
|
|
expect(b.status).toBe(409);
|
|
});
|
|
});
|
|
|
|
describe('POST /api/skills/import', () => {
|
|
it('enforces fileConfig.skills.fileSizeLimit before import handling', async () => {
|
|
mockFileConfig = {
|
|
skills: {
|
|
fileSizeLimit: 1,
|
|
},
|
|
};
|
|
|
|
const res = await request(app)
|
|
.post('/api/skills/import')
|
|
.attach('file', Buffer.alloc(2 * 1024 * 1024), {
|
|
filename: 'too-large.skill',
|
|
contentType: 'application/zip',
|
|
});
|
|
|
|
const { mergeFileConfig } = require('librechat-data-provider');
|
|
expect(mergeFileConfig).toHaveBeenCalledWith(mockFileConfig);
|
|
expect(res.status).toBe(400);
|
|
expect(res.body.error).toMatch(/file too large/i);
|
|
});
|
|
|
|
it('persists storage metadata for imported skill files', async () => {
|
|
const savedFilepath =
|
|
'https://cdn.example.com/r/us-east-2/uploads/user123/imported-script.sh';
|
|
const saveBuffer = jest.fn().mockResolvedValue(savedFilepath);
|
|
const { getFileStrategy } = require('~/server/utils/getFileStrategy');
|
|
const { getStrategyFunctions } = require('~/server/services/Files/strategies');
|
|
getFileStrategy.mockReturnValueOnce('cloudfront');
|
|
getStrategyFunctions.mockReturnValueOnce({ saveBuffer });
|
|
|
|
const zip = new JSZip();
|
|
zip.file(
|
|
'SKILL.md',
|
|
[
|
|
'---',
|
|
'name: imported-skill',
|
|
'description: Imported skill description for route tests.',
|
|
'---',
|
|
'# Imported Skill',
|
|
].join('\n'),
|
|
);
|
|
zip.file('scripts/imported-script.sh', 'echo imported');
|
|
const buffer = await zip.generateAsync({ type: 'nodebuffer' });
|
|
|
|
const res = await request(app).post('/api/skills/import').attach('file', buffer, {
|
|
filename: 'imported-skill.skill',
|
|
contentType: 'application/zip',
|
|
});
|
|
|
|
expect(res.status).toBe(201);
|
|
expect(saveBuffer).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
userId: testUsers.owner._id.toString(),
|
|
basePath: 'uploads',
|
|
}),
|
|
);
|
|
|
|
const savedFile = await SkillFile.findOne({
|
|
relativePath: 'scripts/imported-script.sh',
|
|
}).lean();
|
|
expect(savedFile).toEqual(
|
|
expect.objectContaining({
|
|
filepath: savedFilepath,
|
|
source: 'cloudfront',
|
|
storageKey: 'r/us-east-2/uploads/user123/imported-script.sh',
|
|
storageRegion: 'us-east-2',
|
|
}),
|
|
);
|
|
});
|
|
});
|
|
|
|
describe('GET /api/skills', () => {
|
|
it('returns only skills the caller can access', async () => {
|
|
const mine = await createSkillAsOwner({ name: 'mine-skill' });
|
|
expect(mine.status).toBe(201);
|
|
|
|
setTestUser(testUsers.noAccess);
|
|
const other = await createSkillAsOwner({ name: 'other-skill' });
|
|
expect(other.status).toBe(201);
|
|
// Note: the user middleware grants owner perms to whichever user created, so both
|
|
// users see their own skill only.
|
|
|
|
setTestUser(testUsers.owner);
|
|
const res = await request(app).get('/api/skills');
|
|
expect(res.status).toBe(200);
|
|
expect(mockMaybeRunGitHubSkillSyncForRequest).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
config: expect.objectContaining({ fileStrategy: 'local' }),
|
|
user: expect.objectContaining({ id: testUsers.owner._id.toString() }),
|
|
}),
|
|
);
|
|
expect(res.body.skills.length).toBe(1);
|
|
expect(res.body.skills[0].name).toBe('mine-skill');
|
|
});
|
|
});
|
|
|
|
describe('GET /api/skills/:id', () => {
|
|
it('returns 403 when the user has no access', async () => {
|
|
const created = await createSkillAsOwner();
|
|
expect(created.status).toBe(201);
|
|
setTestUser(testUsers.noAccess);
|
|
const res = await request(app).get(`/api/skills/${created.body._id}`);
|
|
expect(res.status).toBe(403);
|
|
});
|
|
|
|
it('returns the skill to the owner with isPublic flag', async () => {
|
|
const created = await createSkillAsOwner();
|
|
const res = await request(app).get(`/api/skills/${created.body._id}`);
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.name).toBe('demo-skill');
|
|
expect(res.body.isPublic).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe('PATCH /api/skills/:id (optimistic concurrency)', () => {
|
|
it('updates with correct expectedVersion and bumps version', async () => {
|
|
const created = await createSkillAsOwner();
|
|
const res = await request(app)
|
|
.patch(`/api/skills/${created.body._id}`)
|
|
.send({ expectedVersion: 1, description: 'Updated description' });
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.version).toBe(2);
|
|
expect(res.body.description).toBe('Updated description');
|
|
});
|
|
|
|
it('returns 409 on stale expectedVersion', async () => {
|
|
const created = await createSkillAsOwner();
|
|
const first = await request(app)
|
|
.patch(`/api/skills/${created.body._id}`)
|
|
.send({ expectedVersion: 1, description: 'First' });
|
|
expect(first.status).toBe(200);
|
|
|
|
const stale = await request(app)
|
|
.patch(`/api/skills/${created.body._id}`)
|
|
.send({ expectedVersion: 1, description: 'Stale' });
|
|
expect(stale.status).toBe(409);
|
|
expect(stale.body.error).toBe('skill_version_conflict');
|
|
expect(stale.body.current.version).toBe(2);
|
|
});
|
|
|
|
it('rejects updates without expectedVersion', async () => {
|
|
const created = await createSkillAsOwner();
|
|
const res = await request(app)
|
|
.patch(`/api/skills/${created.body._id}`)
|
|
.send({ description: 'no version' });
|
|
expect(res.status).toBe(400);
|
|
});
|
|
|
|
it('returns 403 for a user without EDIT permission', async () => {
|
|
const created = await createSkillAsOwner();
|
|
setTestUser(testUsers.noAccess);
|
|
const res = await request(app)
|
|
.patch(`/api/skills/${created.body._id}`)
|
|
.send({ expectedVersion: 1, description: 'nope' });
|
|
expect(res.status).toBe(403);
|
|
});
|
|
});
|
|
|
|
describe('DELETE /api/skills/:id', () => {
|
|
it('deletes and cascades ACL entries', async () => {
|
|
const created = await createSkillAsOwner();
|
|
const res = await request(app).delete(`/api/skills/${created.body._id}`);
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.deleted).toBe(true);
|
|
|
|
const remainingAcl = await AclEntry.countDocuments({
|
|
resourceType: ResourceType.SKILL,
|
|
resourceId: created.body._id,
|
|
});
|
|
expect(remainingAcl).toBe(0);
|
|
});
|
|
|
|
it('returns 403 for a non-owner', async () => {
|
|
const created = await createSkillAsOwner();
|
|
setTestUser(testUsers.noAccess);
|
|
const res = await request(app).delete(`/api/skills/${created.body._id}`);
|
|
expect(res.status).toBe(403);
|
|
});
|
|
});
|
|
|
|
describe('GET /api/skills/:id/files', () => {
|
|
it('returns an empty list for a skill with no files', async () => {
|
|
const created = await createSkillAsOwner();
|
|
const res = await request(app).get(`/api/skills/${created.body._id}/files`);
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.files).toEqual([]);
|
|
});
|
|
});
|
|
|
|
describe('POST /api/skills/:id/files (live)', () => {
|
|
it('returns 400 when no file is provided', async () => {
|
|
const created = await createSkillAsOwner();
|
|
const res = await request(app).post(`/api/skills/${created.body._id}/files`);
|
|
expect(res.status).toBe(400);
|
|
expect(res.body.error).toMatch(/no file/i);
|
|
});
|
|
});
|
|
|
|
describe('GET /api/skills/:id/files/:relativePath', () => {
|
|
it('returns SKILL.md content from skill body', async () => {
|
|
const created = await createSkillAsOwner();
|
|
const res = await request(app).get(`/api/skills/${created.body._id}/files/SKILL.md`);
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.mimeType).toBe('text/markdown');
|
|
expect(res.body.isBinary).toBe(false);
|
|
expect(res.body.filename).toBe('SKILL.md');
|
|
expect(res.body.content).toBeDefined();
|
|
});
|
|
|
|
it('returns 404 for a nonexistent file', async () => {
|
|
const created = await createSkillAsOwner();
|
|
const res = await request(app).get(
|
|
`/api/skills/${created.body._id}/files/scripts%2Fmissing.sh`,
|
|
);
|
|
expect(res.status).toBe(404);
|
|
});
|
|
});
|
|
|
|
describe('DELETE /api/skills/:id/files/:relativePath', () => {
|
|
const { upsertSkillFile } = require('~/models');
|
|
|
|
it('deletes an existing skill file, bumps skill version, and returns 200', async () => {
|
|
const created = await createSkillAsOwner();
|
|
await upsertSkillFile({
|
|
skillId: created.body._id,
|
|
relativePath: 'scripts/parse.sh',
|
|
file_id: 'file-1',
|
|
filename: 'parse.sh',
|
|
filepath: '/tmp/parse.sh',
|
|
source: 'local',
|
|
mimeType: 'text/x-shellscript',
|
|
bytes: 42,
|
|
author: testUsers.owner._id,
|
|
});
|
|
|
|
const beforeSkill = await request(app).get(`/api/skills/${created.body._id}`);
|
|
expect(beforeSkill.body.fileCount).toBe(1);
|
|
expect(beforeSkill.body.version).toBe(2);
|
|
|
|
const res = await request(app).delete(
|
|
`/api/skills/${created.body._id}/files/scripts%2Fparse.sh`,
|
|
);
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toEqual({
|
|
skillId: created.body._id,
|
|
relativePath: 'scripts/parse.sh',
|
|
deleted: true,
|
|
});
|
|
|
|
const afterSkill = await request(app).get(`/api/skills/${created.body._id}`);
|
|
expect(afterSkill.body.fileCount).toBe(0);
|
|
expect(afterSkill.body.version).toBe(3);
|
|
});
|
|
|
|
it('returns 404 when the file does not exist', async () => {
|
|
const created = await createSkillAsOwner();
|
|
const res = await request(app).delete(
|
|
`/api/skills/${created.body._id}/files/scripts%2Fmissing.sh`,
|
|
);
|
|
expect(res.status).toBe(404);
|
|
});
|
|
|
|
it('returns 403 for a non-owner', async () => {
|
|
const created = await createSkillAsOwner();
|
|
setTestUser(testUsers.noAccess);
|
|
const res = await request(app).delete(
|
|
`/api/skills/${created.body._id}/files/scripts%2Fparse.sh`,
|
|
);
|
|
expect(res.status).toBe(403);
|
|
});
|
|
});
|
|
|
|
describe('Sharing via ACL (editor grant)', () => {
|
|
it('allows an editor to patch a shared skill', async () => {
|
|
const created = await createSkillAsOwner();
|
|
await grantPermission({
|
|
principalType: PrincipalType.USER,
|
|
principalId: testUsers.editor._id,
|
|
resourceType: ResourceType.SKILL,
|
|
resourceId: created.body._id,
|
|
accessRoleId: AccessRoleIds.SKILL_EDITOR,
|
|
grantedBy: testUsers.owner._id,
|
|
});
|
|
|
|
setTestUser(testUsers.editor);
|
|
const res = await request(app)
|
|
.patch(`/api/skills/${created.body._id}`)
|
|
.send({ expectedVersion: 1, description: 'Edited by editor' });
|
|
expect(res.status).toBe(200);
|
|
|
|
// Editor should NOT be able to delete
|
|
const del = await request(app).delete(`/api/skills/${created.body._id}`);
|
|
expect(del.status).toBe(403);
|
|
});
|
|
});
|
|
});
|