feat: allow user override of hardcoded disallowed tools (#71)
* feat: allow user override of hardcoded disallowed tools Allow users to override hardcoded disallowed tools (WebSearch, WebFetch) by including them in their allowed_tools configuration. This provides users with the ability to control tool access based on their security requirements. Changes: - Modified buildDisallowedToolsString() to accept allowedTools parameter - Added logic to filter out hardcoded disallowed tools if present in allowed tools - Updated function call site to pass allowedTools - Added comprehensive test coverage for override behavior - Maintains backward compatibility Resolves #49 Co-authored-by: ashwin-ant <ashwin-ant@users.noreply.github.com> * prettier --------- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: ashwin-ant <ashwin-ant@users.noreply.github.com>
This commit is contained in:
@@ -58,10 +58,27 @@ export function buildAllowedToolsString(
|
|||||||
|
|
||||||
export function buildDisallowedToolsString(
|
export function buildDisallowedToolsString(
|
||||||
customDisallowedTools?: string,
|
customDisallowedTools?: string,
|
||||||
|
allowedTools?: string,
|
||||||
): string {
|
): string {
|
||||||
let allDisallowedTools = DISALLOWED_TOOLS.join(",");
|
let disallowedTools = [...DISALLOWED_TOOLS];
|
||||||
|
|
||||||
|
// If user has explicitly allowed some hardcoded disallowed tools, remove them from disallowed list
|
||||||
|
if (allowedTools) {
|
||||||
|
const allowedToolsArray = allowedTools
|
||||||
|
.split(",")
|
||||||
|
.map((tool) => tool.trim());
|
||||||
|
disallowedTools = disallowedTools.filter(
|
||||||
|
(tool) => !allowedToolsArray.includes(tool),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
let allDisallowedTools = disallowedTools.join(",");
|
||||||
if (customDisallowedTools) {
|
if (customDisallowedTools) {
|
||||||
|
if (allDisallowedTools) {
|
||||||
allDisallowedTools = `${allDisallowedTools},${customDisallowedTools}`;
|
allDisallowedTools = `${allDisallowedTools},${customDisallowedTools}`;
|
||||||
|
} else {
|
||||||
|
allDisallowedTools = customDisallowedTools;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return allDisallowedTools;
|
return allDisallowedTools;
|
||||||
}
|
}
|
||||||
@@ -648,6 +665,7 @@ export async function createPrompt(
|
|||||||
);
|
);
|
||||||
const allDisallowedTools = buildDisallowedToolsString(
|
const allDisallowedTools = buildDisallowedToolsString(
|
||||||
preparedContext.disallowedTools,
|
preparedContext.disallowedTools,
|
||||||
|
preparedContext.allowedTools,
|
||||||
);
|
);
|
||||||
|
|
||||||
core.exportVariable("ALLOWED_TOOLS", allAllowedTools);
|
core.exportVariable("ALLOWED_TOOLS", allAllowedTools);
|
||||||
|
|||||||
@@ -722,4 +722,51 @@ describe("buildDisallowedToolsString", () => {
|
|||||||
expect(parts).toContain("BadTool1");
|
expect(parts).toContain("BadTool1");
|
||||||
expect(parts).toContain("BadTool2");
|
expect(parts).toContain("BadTool2");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("should remove hardcoded disallowed tools if they are in allowed tools", () => {
|
||||||
|
const customDisallowedTools = "BadTool1,BadTool2";
|
||||||
|
const allowedTools = "WebSearch,SomeOtherTool";
|
||||||
|
const result = buildDisallowedToolsString(
|
||||||
|
customDisallowedTools,
|
||||||
|
allowedTools,
|
||||||
|
);
|
||||||
|
|
||||||
|
// WebSearch should be removed from disallowed since it's in allowed
|
||||||
|
expect(result).not.toContain("WebSearch");
|
||||||
|
|
||||||
|
// WebFetch should still be disallowed since it's not in allowed
|
||||||
|
expect(result).toContain("WebFetch");
|
||||||
|
|
||||||
|
// Custom disallowed tools should still be present
|
||||||
|
expect(result).toContain("BadTool1");
|
||||||
|
expect(result).toContain("BadTool2");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("should remove all hardcoded disallowed tools if they are all in allowed tools", () => {
|
||||||
|
const allowedTools = "WebSearch,WebFetch,SomeOtherTool";
|
||||||
|
const result = buildDisallowedToolsString(undefined, allowedTools);
|
||||||
|
|
||||||
|
// Both hardcoded disallowed tools should be removed
|
||||||
|
expect(result).not.toContain("WebSearch");
|
||||||
|
expect(result).not.toContain("WebFetch");
|
||||||
|
|
||||||
|
// Result should be empty since no custom disallowed tools provided
|
||||||
|
expect(result).toBe("");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("should handle custom disallowed tools when all hardcoded tools are overridden", () => {
|
||||||
|
const customDisallowedTools = "BadTool1,BadTool2";
|
||||||
|
const allowedTools = "WebSearch,WebFetch";
|
||||||
|
const result = buildDisallowedToolsString(
|
||||||
|
customDisallowedTools,
|
||||||
|
allowedTools,
|
||||||
|
);
|
||||||
|
|
||||||
|
// Hardcoded tools should be removed
|
||||||
|
expect(result).not.toContain("WebSearch");
|
||||||
|
expect(result).not.toContain("WebFetch");
|
||||||
|
|
||||||
|
// Only custom disallowed tools should remain
|
||||||
|
expect(result).toBe("BadTool1,BadTool2");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user