fix(security): harden skill apply and activation flows
Restrict auto-apply to trusted review comments so spoofed issue comments cannot write optimized SKILL.md content into pull request branches. Reject activation symlinks that escape the source root and add regression coverage for both security checks.
This commit is contained in:
@@ -5,6 +5,8 @@ const path = require('node:path');
|
||||
const { execFileSync } = require('node:child_process');
|
||||
|
||||
const REVIEW_HEADING = '## Tessl Skill Review';
|
||||
const TRUSTED_AUTHOR_ASSOCIATIONS = new Set(['OWNER', 'MEMBER', 'COLLABORATOR']);
|
||||
const DEFAULT_ALLOWED_REVIEW_BOT_LOGINS = ['github-actions[bot]'];
|
||||
|
||||
function getRequiredEnv(name) {
|
||||
const value = process.env[name];
|
||||
@@ -88,7 +90,46 @@ function ensureRepoRelative(filePath) {
|
||||
return resolved;
|
||||
}
|
||||
|
||||
async function findLatestReviewComment(owner, repo, prNumber) {
|
||||
function parseAllowedReviewBotLogins(envValue = process.env.APPLY_OPTIMIZE_ALLOWED_REVIEW_BOT_LOGINS) {
|
||||
if (!envValue) {
|
||||
return DEFAULT_ALLOWED_REVIEW_BOT_LOGINS;
|
||||
}
|
||||
|
||||
const logins = envValue
|
||||
.split(',')
|
||||
.map((item) => item.trim())
|
||||
.filter(Boolean);
|
||||
|
||||
return logins.length > 0 ? logins : DEFAULT_ALLOWED_REVIEW_BOT_LOGINS;
|
||||
}
|
||||
|
||||
function isTrustedReviewComment(comment, allowedBotLogins = DEFAULT_ALLOWED_REVIEW_BOT_LOGINS) {
|
||||
if (!comment?.body || !comment.body.includes(REVIEW_HEADING)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (TRUSTED_AUTHOR_ASSOCIATIONS.has(comment.author_association)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const login = comment.user?.login;
|
||||
const userType = comment.user?.type;
|
||||
return Boolean(login && userType === 'Bot' && allowedBotLogins.includes(login));
|
||||
}
|
||||
|
||||
function selectLatestTrustedReviewComment(comments, allowedBotLogins = DEFAULT_ALLOWED_REVIEW_BOT_LOGINS) {
|
||||
let latest = null;
|
||||
|
||||
for (const comment of comments) {
|
||||
if (isTrustedReviewComment(comment, allowedBotLogins)) {
|
||||
latest = comment;
|
||||
}
|
||||
}
|
||||
|
||||
return latest;
|
||||
}
|
||||
|
||||
async function findLatestTrustedReviewComment(owner, repo, prNumber, allowedBotLogins) {
|
||||
let page = 1;
|
||||
let latest = null;
|
||||
|
||||
@@ -97,11 +138,7 @@ async function findLatestReviewComment(owner, repo, prNumber) {
|
||||
`/repos/${owner}/${repo}/issues/${prNumber}/comments?per_page=100&page=${page}`,
|
||||
);
|
||||
|
||||
for (const comment of comments) {
|
||||
if (comment.body && comment.body.includes(REVIEW_HEADING)) {
|
||||
latest = comment;
|
||||
}
|
||||
}
|
||||
latest = selectLatestTrustedReviewComment(comments, allowedBotLogins) ?? latest;
|
||||
|
||||
if (comments.length < 100) {
|
||||
return latest;
|
||||
@@ -133,6 +170,7 @@ async function main() {
|
||||
}
|
||||
|
||||
const pr = await githubRequest(`/repos/${owner}/${repo}/pulls/${prNumber}`);
|
||||
const allowedReviewBotLogins = parseAllowedReviewBotLogins();
|
||||
|
||||
if (pr.head.repo.full_name !== repoSlug) {
|
||||
throw new Error(
|
||||
@@ -140,9 +178,16 @@ async function main() {
|
||||
);
|
||||
}
|
||||
|
||||
const reviewComment = await findLatestReviewComment(owner, repo, prNumber);
|
||||
const reviewComment = await findLatestTrustedReviewComment(
|
||||
owner,
|
||||
repo,
|
||||
prNumber,
|
||||
allowedReviewBotLogins,
|
||||
);
|
||||
if (!reviewComment || !reviewComment.body) {
|
||||
throw new Error('No Tessl skill review comment found on this PR. Run the review first.');
|
||||
throw new Error(
|
||||
'No trusted Tessl skill review comment found on this PR. Run the review first.',
|
||||
);
|
||||
}
|
||||
|
||||
const optimizedFiles = extractOptimizedContent(reviewComment.body);
|
||||
@@ -205,7 +250,18 @@ async function main() {
|
||||
await postComment(owner, repo, prNumber, body);
|
||||
}
|
||||
|
||||
main().catch((error) => {
|
||||
console.error(error instanceof Error ? error.stack : String(error));
|
||||
process.exitCode = 1;
|
||||
});
|
||||
if (require.main === module) {
|
||||
main().catch((error) => {
|
||||
console.error(error instanceof Error ? error.stack : String(error));
|
||||
process.exitCode = 1;
|
||||
});
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
DEFAULT_ALLOWED_REVIEW_BOT_LOGINS,
|
||||
REVIEW_HEADING,
|
||||
findLatestTrustedReviewComment,
|
||||
isTrustedReviewComment,
|
||||
parseAllowedReviewBotLogins,
|
||||
selectLatestTrustedReviewComment,
|
||||
};
|
||||
|
||||
@@ -10,6 +10,7 @@ const scriptPath = path.join(repoRoot, "scripts", "activate-skills.sh");
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), "activate-skills-shell-"));
|
||||
const baseDir = path.join(root, "antigravity");
|
||||
const repoSkills = path.join(root, "repo-skills");
|
||||
const outsideDir = path.join(root, "outside-skill-root");
|
||||
|
||||
function makeSkill(skillId) {
|
||||
const skillDir = path.join(repoSkills, skillId);
|
||||
@@ -25,6 +26,9 @@ try {
|
||||
makeSkill("brainstorming");
|
||||
makeSkill("systematic-debugging");
|
||||
makeSkill("custom-skill");
|
||||
fs.mkdirSync(outsideDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(outsideDir, "secret.txt"), "outside", "utf8");
|
||||
fs.symlinkSync(outsideDir, path.join(repoSkills, "escape-link"), "dir");
|
||||
|
||||
const result = spawnSync(
|
||||
"bash",
|
||||
@@ -54,6 +58,14 @@ try {
|
||||
fs.existsSync(path.join(baseDir, "skills_library", "brainstorming", "SKILL.md")),
|
||||
"repo skills should be synced into the backing library",
|
||||
);
|
||||
assert.ok(
|
||||
!fs.existsSync(path.join(baseDir, "skills_library", "escape-link")),
|
||||
"repo sync must not copy symlinked skills that point outside the source root",
|
||||
);
|
||||
assert.ok(
|
||||
!fs.existsSync(path.join(baseDir, "skills", "escape-link")),
|
||||
"unsafe symlinked skills must never become active",
|
||||
);
|
||||
assert.match(
|
||||
result.stdout,
|
||||
/Done! Antigravity skills are now activated\./,
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
const assert = require('node:assert');
|
||||
const path = require('node:path');
|
||||
|
||||
const repoRoot = path.resolve(__dirname, '..', '..', '..');
|
||||
const {
|
||||
selectLatestTrustedReviewComment,
|
||||
} = require(path.join(repoRoot, 'tools', 'scripts', 'apply_skill_optimization.cjs'));
|
||||
|
||||
const trustedMaintainerComment = {
|
||||
body: '## Tessl Skill Review\ntrusted maintainer content',
|
||||
author_association: 'MEMBER',
|
||||
user: { login: 'maintainer-user', type: 'User' },
|
||||
};
|
||||
|
||||
const spoofedUserComment = {
|
||||
body: '## Tessl Skill Review\nspoofed content',
|
||||
author_association: 'NONE',
|
||||
user: { login: 'random-user', type: 'User' },
|
||||
};
|
||||
|
||||
const trustedBotComment = {
|
||||
body: '## Tessl Skill Review\nbot content',
|
||||
author_association: 'NONE',
|
||||
user: { login: 'github-actions[bot]', type: 'Bot' },
|
||||
};
|
||||
|
||||
assert.strictEqual(
|
||||
selectLatestTrustedReviewComment([trustedMaintainerComment, spoofedUserComment]),
|
||||
trustedMaintainerComment,
|
||||
'the apply script must ignore later untrusted review comments',
|
||||
);
|
||||
|
||||
assert.strictEqual(
|
||||
selectLatestTrustedReviewComment([spoofedUserComment, trustedBotComment]),
|
||||
trustedBotComment,
|
||||
'the apply script should accept review comments from the configured trusted bot',
|
||||
);
|
||||
|
||||
assert.strictEqual(
|
||||
selectLatestTrustedReviewComment([spoofedUserComment]),
|
||||
null,
|
||||
'the apply script must reject untrusted review comments entirely',
|
||||
);
|
||||
@@ -11,6 +11,7 @@ const LOCAL_TEST_COMMANDS = [
|
||||
[path.join(TOOL_TESTS, "activate_skills_shell.test.js")],
|
||||
[path.join(TOOL_TESTS, "activate_skills_batch_security.test.js")],
|
||||
[path.join(TOOL_TESTS, "automation_workflows.test.js")],
|
||||
[path.join(TOOL_TESTS, "apply_skill_optimization_security.test.js")],
|
||||
[path.join(TOOL_TESTS, "build_catalog_bundles.test.js")],
|
||||
[path.join(TOOL_TESTS, "claude_plugin_marketplace.test.js")],
|
||||
[path.join(TOOL_TESTS, "installer_antigravity_guidance.test.js")],
|
||||
|
||||
Reference in New Issue
Block a user