fix(auth): protect builtin system account boundaries

Signed-off-by: dongmucat <1127093059@qq.com>
This commit is contained in:
dongmucat 2026-06-08 18:01:20 +08:00
parent 5cc934a294
commit 973c336c82
5 changed files with 52 additions and 1 deletions

View file

@ -142,7 +142,7 @@ skillhub-hello-1.0.0.zip
3. 查询 `@global` 命名空间是否存在;如果不存在,跳过同步。
4. 确保系统发布者 `builtin-skill-publisher` 存在,并且该账号带有系统账号标记。
5. 如果该用户 ID 已被非系统账号占用,直接跳过本次内置 Skill 同步,不授予 `@global` 权限。
6. 确保系统发布者是 `@global` 的 `OWNER`。
6. 如果系统发布者还不是 `@global` 成员,则创建 `OWNER` 成员记录;已有成员记录不会自动改角色。
7. 按 manifest 顺序处理每一个 item。
8. 下载前先检查 `@global/{slug}` 和目标版本是否已经存在;如果已经确定应跳过,则不发起远程下载。
9. 只有需要发布新 Skill 或新版本时,才下载对应 zip 包。

View file

@ -21,4 +21,14 @@ WHERE id = 'builtin-skill-publisher'
SELECT 1
FROM api_token
WHERE api_token.user_id = user_account.id
)
AND NOT EXISTS (
SELECT 1
FROM user_role_binding
WHERE user_role_binding.user_id = user_account.id
)
AND NOT EXISTS (
SELECT 1
FROM namespace_member
WHERE namespace_member.user_id = user_account.id
);

View file

@ -80,6 +80,16 @@ class FlywayMigrationGuardrailTest {
assertThat(migration).contains("api_token.user_id = user_account.id");
}
@Test
void systemAccountMigration_mustNotPromoteUsersWithRolesOrNamespaceMemberships() throws IOException {
String migration = Files.readString(migrationPath("V43__user_account_system_account.sql"));
assertThat(migration).contains("FROM user_role_binding");
assertThat(migration).contains("user_role_binding.user_id = user_account.id");
assertThat(migration).contains("FROM namespace_member");
assertThat(migration).contains("namespace_member.user_id = user_account.id");
}
private List<Path> migrationFiles() throws IOException {
Path root = repoRoot()
.resolve("server")

View file

@ -178,6 +178,9 @@ public class PasswordResetService {
}
private boolean isEligibleForReset(UserAccount user) {
if (user.isSystemAccount()) {
return false;
}
if (user.getStatus() != UserStatus.ACTIVE) {
return false;
}

View file

@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.atLeastOnce;
@ -215,4 +216,31 @@ class PasswordResetServiceTest {
.extracting("status")
.isEqualTo(HttpStatus.BAD_REQUEST);
}
@Test
void adminTriggerPasswordReset_forSystemAccount_throwsBadRequest() {
UserAccount user = UserAccount.systemAccount(
"builtin-skill-publisher",
"Built-in Skill Publisher",
"builtin@example.com",
null
);
given(userAccountRepository.findById("builtin-skill-publisher")).willReturn(Optional.of(user));
lenient().when(credentialRepository.findByUserId("builtin-skill-publisher")).thenReturn(
Optional.of(new LocalCredential("builtin-skill-publisher", "builtin", "encoded"))
);
lenient().when(resetRequestRepository.findByUserIdAndConsumedAtIsNullAndExpiresAtAfterOrderByCreatedAtDesc(
anyString(), any(Instant.class))
).thenReturn(List.of());
lenient().when(passwordEncoder.encode(anyString())).thenReturn("encoded-value");
assertThatThrownBy(() -> service.adminTriggerPasswordReset("builtin-skill-publisher", "admin_1"))
.isInstanceOf(AuthFlowException.class)
.extracting("status")
.isEqualTo(HttpStatus.BAD_REQUEST);
verify(credentialRepository, never()).findByUserId("builtin-skill-publisher");
verify(resetRequestRepository, never()).save(any(PasswordResetRequest.class));
verify(mailSender, never()).send(any(SimpleMailMessage.class));
}
}