fix: Guard existing skill sync permission grants

This commit is contained in:
Danny Avila 2026-06-01 19:09:29 -04:00
parent 90ed7dea40
commit d79712bf5d
2 changed files with 40 additions and 3 deletions

View file

@ -1069,6 +1069,44 @@ describe('createGitHubSkillSyncRunner', () => {
expect(deps.grantPermission).toHaveBeenCalled();
});
it('does not mutate existing skill files when permission grant fails', async () => {
const existing = makeSkill({
name: 'research',
description: 'Old description',
body: 'Old body',
author: new Types.ObjectId(),
authorName: 'GitHub Sync',
source: 'github',
sourceMetadata: {
provider: 'github',
sourceId: 'librechat-skills',
upstreamId: 'librechat-skills:skills/research',
owner: 'LibreChat',
repo: 'skills',
ref: 'main',
skillPath: 'skills/research',
},
}) as ISkill & { _id: Types.ObjectId };
const deps = createDeps({
findSkillBySourceIdentity: jest.fn(async () => existing),
getSkillById: jest.fn(async () => existing),
grantPermission: jest.fn(async () => {
throw new Error('permission unavailable');
}),
});
const runner = createGitHubSkillSyncRunner(deps);
const result = await runner.runOnce();
expect(result.status).toBe('failed');
expect(deps.grantPermission).toHaveBeenCalledWith(
expect.objectContaining({ resourceId: existing._id }),
);
expect(deps.listSkillFiles).not.toHaveBeenCalled();
expect(deps.saveBuffer).not.toHaveBeenCalled();
expect(deps.upsertSkillFile).not.toHaveBeenCalled();
expect(deps.updateSkill).not.toHaveBeenCalled();
});
it('restores existing skill files when the skill update fails after file sync', async () => {
const existing = makeSkill({
name: 'research',

View file

@ -1276,10 +1276,10 @@ async function syncSource(params: {
`Skill "${effectivePrepared.existing.name}" was modified during sync`,
);
}
await ensurePublicViewer(deps, effectivePrepared.existing._id);
const previousFiles = await deps.listSkillFiles(effectivePrepared.existing._id);
const journal: SyncSkillFilesJournal = { staleFiles: [], savedFiles: [] };
let fileCounts: SyncSkillFilesResult;
let upserted: UpsertRemoteSkillResult;
try {
fileCounts = await syncSkillFiles({
deps,
@ -1292,7 +1292,7 @@ async function syncSource(params: {
assertNotCancelled,
journal,
});
upserted = await commitExistingRemoteSkillAfterFileSync(
await commitExistingRemoteSkillAfterFileSync(
deps,
{
...effectivePrepared,
@ -1314,7 +1314,6 @@ async function syncSource(params: {
);
throw error;
}
await ensurePublicViewer(deps, upserted.skill._id);
await cleanupStoredFiles({
deps,
files: fileCounts.staleFiles,