🧭 fix: Restore Empty Skill Allowlist Catalog (#13526)

This commit is contained in:
Danny Avila 2026-06-05 12:30:48 -04:00 committed by GitHub
parent 2c8d54e18c
commit 5118a566df
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 36 additions and 10 deletions

View file

@ -77,10 +77,9 @@ const sanitizeViewerSkillScope = (agent, accessibleSkillSet) => {
const configuredSkills = Array.isArray(agent.skills) ? agent.skills : [];
if (configuredSkills.length === 0) {
// Empty allowlist means the viewer's full accessible catalog.
delete agent.skills;
if (accessibleSkillSet.size > 0) {
agent.skills_enabled = true;
}
agent.skills_enabled = true;
return agent;
}

View file

@ -1548,6 +1548,33 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
expect(response.data[0].skills_enabled).toBeUndefined();
});
test('should preserve enabled skill scope for VIEW list callers with an empty allowlist', async () => {
await Agent.findByIdAndUpdate(agentA1._id, {
skills_enabled: true,
skills: [],
});
mockReq.user.id = userB.toString();
mockReq.query.requiredPermission = String(PermissionBits.VIEW);
findAccessibleResources.mockImplementation(({ resourceType }) => {
if (resourceType === ResourceType.AGENT) {
return Promise.resolve([agentA1._id]);
}
if (resourceType === ResourceType.SKILL) {
return Promise.resolve([]);
}
return Promise.resolve([]);
});
findPubliclyAccessibleResources.mockResolvedValue([]);
await getListAgentsHandler(mockReq, mockRes);
const response = mockRes.json.mock.calls[0][0];
expect(response.data).toHaveLength(1);
expect(response.data[0].skills).toBeUndefined();
expect(response.data[0].skills_enabled).toBe(true);
});
test('should return raw skill configuration for EDIT list callers', async () => {
const visibleSkillId = new mongoose.Types.ObjectId();
const hiddenSkillId = new mongoose.Types.ObjectId();

View file

@ -320,7 +320,7 @@ describe('SkillsCommand', () => {
isFetchingNextPage: false,
});
mockUseAgentsMapContext.mockReturnValue({
agent_1: { id: 'agent_1', skills_enabled: true },
agent_1: { id: 'agent_1', skills: [], skills_enabled: true },
});
const textAreaRef = makeTextarea('$');

View file

@ -253,9 +253,9 @@ describe('scopeSkillIds', () => {
expect(scopeSkillIds(accessible, null)).toBe(accessible);
});
it('returns [] when agentSkills is an empty array (explicit none)', () => {
it('returns the full set when agentSkills is an empty array (no allowlist)', () => {
const accessible = [makeId(), makeId()];
expect(scopeSkillIds(accessible, [])).toEqual([]);
expect(scopeSkillIds(accessible, [])).toBe(accessible);
});
it('returns intersection when agentSkills overlaps accessibleSkillIds', () => {

View file

@ -101,9 +101,9 @@ export function isSkillPrimeMessage(msg: unknown): boolean {
*
* Semantics (pinned by unit tests):
* - `undefined` / `null` not configured, returns the full accessible catalog.
* - `[]` (empty array) explicitly none, returns `[]`. A user who narrows their
* agent to a subset and then removes all entries is explicitly opting out of
* the full catalog fallback.
* - `[]` (empty array) no allowlist, returns the full accessible catalog.
* Removing all selected skills in the builder restores the default full-catalog
* behavior while `skills_enabled` remains true.
* - non-empty array of skill `_id` hex strings intersection of accessible IDs
* and agent-configured IDs.
*
@ -120,7 +120,7 @@ export function scopeSkillIds(
return accessibleSkillIds;
}
if (agentSkills.length === 0) {
return [];
return accessibleSkillIds;
}
const agentSet = new Set(agentSkills);
return accessibleSkillIds.filter((oid) => agentSet.has(oid.toString()));