Skip to content

Commit fb5033b

Browse files
committed
fix(security): Copilot CR 反馈 6 处
PR #26 Copilot review 7 条评论里 6 条本 commit 处理,第 7 条(chat saveTurn 的 TOCTOU window)单独跟踪 follow-up,本 PR scope 不修。 修复点: 1. SECURITY.md INV-004 引用的测试方法名跟实际代码对不上——文档本来写 `user_follows表存在且follow_unfollow能闭环`,实际是 `user_follows表存 在且字段可读写`,按 Copilot 反馈对齐文档。 2. test-schema.sql 之前只建了 user_follows 表但没建 idx_user_follows_followee 索引,跟生产 schema.sql 漂移;补上 CREATE INDEX 让"索引被误删"也能被 CI 抓到(即便表本身仍存在)。 3. INV-005a compose 端口扫描原来只盯 "5432:5432" / "0.0.0.0:5432:5432" 两种带引号写法,漏过无引号 5432:5432、IPv6 [::]:5432、host port 改成 15432 但容器仍 5432 等其它形式。改成扫描所有 host:5432 mapping,凡是 host part 不以 127.0.0.1: 起头的全算违规。 4. AuthService lazy upgrade 失败的 log.warn 之前只传 e.getMessage(),丢 stack trace;改成把异常对象作为最后一个参数,保留完整堆栈用于排查 死锁/连接池/权限根因,"不阻断登录"行为不变。 5. UserCenterService.updateAuthorization 之前只检查 RESTRICTED_ROLES, 没拒 null/blank 元素——roles 里塞 null 会让 repository 的 String.join 抛 NPE 走 500 兜底。增加 null/blank 校验返回 400,permissions 同样补齐。 6. 新增 3 条 AuthServiceTests 覆盖 INV-003 lazy upgrade 路径: - legacy hash 匹配后触发 updatePasswordHash 写回 - updatePasswordHash 抛异常时不阻断登录 - bcrypt hash 路径不触发 lazy upgrade 测试:./mvnw test 全套 215 / 215(之前 212 + 新增 3 条)。 未在本 PR 修: - ChatHistoryController#save 的 lookupOwner + saveTurn 两步 TOCTOU window: 利用窗口需要 victim/attacker 毫秒级并发写同一 UUID chatId,最坏后果 一条 message 错挂到 victim chat。完整修法是把逻辑下沉到 service 层 @transactional + 改 saveTurn SQL 加 owner WHERE,工作量超出本 hotfix scope,单独 follow-up 跟踪。
1 parent f40e1e0 commit fb5033b

6 files changed

Lines changed: 129 additions & 16 deletions

File tree

‎SECURITY.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,15 @@
8686
- `docker/init-db/init.sql`(同步 DDL)
8787
- `src/test/resources/test-schema.sql`(测试同步)
8888
- `usercenter/follows/FollowService`(SQL 调用方)
89-
- **测试**:`SecurityInvariantsTests#user_follows表存在且follow_unfollow能闭环`
89+
- **测试**:`SecurityInvariantsTests#user_follows表存在且字段可读写`
90+
(不直接调 FollowService.follow() 端到端,因 FollowService 用 PG 专属
91+
`ON CONFLICT (cols) DO NOTHING`,H2 PostgreSQL MODE 当前版本不识别——
92+
改打 JdbcTemplate 直读直写表,验证 schema drift 这一根因)
9093
- **为什么**:FollowService 引用的 `user_follows(follower_id, followee_id, created_at)`
9194
之前在 schema.sql 与 init.sql 中都未定义,全新部署或新 Neon 数据库一访问
9295
`/api/user-center/follows/...` 就 `relation does not exist` 报 500。
93-
这是 issue #25 之外的额外 schema drift。CI 测试存在表 + follow/unfollow 闭环
94-
作为长期回归探测器。
96+
这是 issue #25 之外的额外 schema drift。CI 测试存在表 + 字段可读写作为
97+
长期回归探测器。
9598
- **历史**:2026-05-07 三方 CR 发现的 schema drift(与 issue #25 类同)。
9699

97100
---

‎src/main/java/com/involutionhell/backend/usercenter/service/AuthService.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,10 @@ public LoginResponse login(LoginRequest request) {
6060
passwordService.hash(request.password()));
6161
log.info("已就地升级用户 {} 的密码哈希(legacy → bcrypt)", userAccount.username());
6262
} catch (Exception e) {
63-
// lazy upgrade 失败不阻断登录——记日志即可,下次登录会再次尝试
64-
log.warn("用户 {} 密码哈希升级失败:{}", userAccount.username(), e.getMessage());
63+
// lazy upgrade 失败不阻断登录——记日志即可,下次登录会再次尝试。
64+
// 必须把异常对象作为最后一个参数传给 SLF4J 才能打完整堆栈,
65+
// 仅传 e.getMessage() 会丢失死锁/连接池/权限等根因排查线索。
66+
log.warn("用户 {} 密码哈希升级失败", userAccount.username(), e);
6567
}
6668
}
6769

‎src/main/java/com/involutionhell/backend/usercenter/service/UserCenterService.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,29 @@ public UserView getUser(Long userId) {
114114
*/
115115
public UserView updateAuthorization(Long userId, UserAuthorizationUpdateRequest request) {
116116
Set<String> requestedRoles = request.roles() == null ? Set.of() : request.roles();
117+
// 顺序校验:先拒非法元素(null / 空白),再拒受限角色。
118+
// 不修这层:requestedRoles 里有 null 元素会让 repository 的 String.join 抛 NPE
119+
// 走 Exception.class 兜底 500,而不是 400,并丢失"哪个字段错了"信息。
117120
for (String role : requestedRoles) {
121+
if (role == null || role.isBlank()) {
122+
throw new IllegalArgumentException("角色名不允许为空或仅含空白字符");
123+
}
118124
if (RESTRICTED_ROLES.contains(role)) {
119125
throw new IllegalArgumentException("不允许通过本接口授予角色: " + role);
120126
}
121127
}
128+
// permissions 同理校验,避免 String.join 触发 NPE
129+
Set<String> requestedPermissions = request.permissions() == null ? Set.of() : request.permissions();
130+
for (String permission : requestedPermissions) {
131+
if (permission == null || permission.isBlank()) {
132+
throw new IllegalArgumentException("权限名不允许为空或仅含空白字符");
133+
}
134+
}
122135

123136
UserAccount updatedAccount = userAccountRepository.updateAuthorization(
124137
userId,
125138
requestedRoles,
126-
request.permissions()
139+
requestedPermissions
127140
);
128141
return UserView.from(updatedAccount);
129142
}

‎src/test/java/com/involutionhell/backend/security/SecurityInvariantsTests.java‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -290,23 +290,41 @@ class SecurityInvariantsTests extends AbstractWebIntegrationTest {
290290
// ============================================================
291291

292292
/**
293-
* INV-005a:docker-compose.yml 里 postgres 服务的 ports 段必须以 127.0.0.1 起头
294-
* 或不存在(容器间走 docker network 即可)。
293+
* INV-005a:docker-compose.yml 里 postgres 容器端口 5432 必须只绑 127.0.0.1。
295294
*
296-
* 防止有人为了"本地连数据库方便"把端口改成 5432:5432 或 0.0.0.0:5432:5432
297-
* —— 一旦上线就直接对外暴露 PostgreSQL 主端口,等同于公网开 5432 蜜罐。
295+
* 攻击场景:把端口写成 "5432:5432" / "0.0.0.0:5432:5432" / "15432:5432"
296+
* 等任何"非 127.0.0.1: 开头"的形式,宿主机就把容器 5432 暴露给公网。
297+
*
298+
* 检测策略:扫描所有 ports 段 mapping 行,凡是 host port 段不以 127.0.0.1:
299+
* 起头且 container port 段为 5432 的,全判违规。覆盖:
300+
* - 引号 / 无引号
301+
* - host 改非 5432(如 15432)映射到容器 5432 仍暴露
302+
* - 0.0.0.0 / ::(IPv6 全绑)等显式绑全网
298303
*/
299304
@Test
300305
void docker_compose里postgres端口必须绑127_0_0_1() throws Exception {
301306
Path composeFile = locateComposeFile();
302307
String content = Files.readString(composeFile);
303308

304-
// 简化版扫描:postgres ports 段后面紧跟的 mapping 行必须 127.0.0.1: 开头
305-
Pattern unsafePattern = Pattern.compile(
306-
"(?m)^\\s*-\\s*\"(0\\.0\\.0\\.0:|\\d+:)5432:5432\"");
307-
Assertions.assertThat(unsafePattern.matcher(content).find())
308-
.as("docker-compose.yml 检测到 postgres 端口暴露公网形式(如 \"5432:5432\" 或 \"0.0.0.0:5432:5432\")")
309-
.isFalse();
309+
// (?m) 行模式;匹配所有 "- <hostpart>:5432" 形式的 ports mapping。
310+
// 捕获组 1 是 hostpart(可能是 "127.0.0.1:5432"、"5432"、"0.0.0.0:5432" 等)。
311+
Pattern portMapping = Pattern.compile(
312+
"(?m)^\\s*-\\s*\"?([^\"\\s#]+):5432\"?\\s*(?:#.*)?$");
313+
java.util.regex.Matcher m = portMapping.matcher(content);
314+
java.util.List<String> violations = new java.util.ArrayList<>();
315+
while (m.find()) {
316+
String hostPart = m.group(1);
317+
// 合规形式:以 "127.0.0.1:" 起头(如 "127.0.0.1:5432")。
318+
// 任何其他形式都不允许:纯端口号 "5432"、"0.0.0.0:..."、IPv6 "[::]:..."、
319+
// "15432" 这种 host port 改了但容器仍是 5432 也算(仍暴露公网)。
320+
if (!hostPart.startsWith("127.0.0.1:")) {
321+
violations.add(m.group(0).trim());
322+
}
323+
}
324+
Assertions.assertThat(violations)
325+
.as("docker-compose.yml postgres 容器 5432 端口必须只绑 127.0.0.1;"
326+
+ "检测到非 loopback 映射: %s", violations)
327+
.isEmpty();
310328
}
311329

312330
/**

‎src/test/java/com/involutionhell/backend/usercenter/service/AuthServiceTests.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,79 @@ void loginThrowsWhenPasswordIsWrong() {
128128
.hasMessage("用户名或密码错误");
129129
}
130130

131+
/**
132+
* INV-003 lazy upgrade:legacy SHA-256 hash 用户登录成功后,应被就地升级为 bcrypt。
133+
*/
134+
@Test
135+
void loginUpgradesLegacyHashAfterSuccessfulMatch() {
136+
String legacyHash = "ad89b64d66caa8e30e5d5ce4a9763f4ecc205814c412175f3e2c50027471426d";
137+
String newBcryptHash = "$2b$10$newbcryptsaltsaltsaltsaltsaltsaltsaltsaltsaltsaltsalts";
138+
UserAccount account = enabledUser(1L, "alice", legacyHash);
139+
when(userCenterService.findByUsername("alice")).thenReturn(Optional.of(account));
140+
when(passwordService.matches("Alice@123", legacyHash)).thenReturn(true);
141+
when(passwordService.isLegacyHash(legacyHash)).thenReturn(true);
142+
when(passwordService.hash("Alice@123")).thenReturn(newBcryptHash);
143+
144+
try (MockedStatic<StpUtil> stpUtil = mockStatic(StpUtil.class)) {
145+
stpUtil.when(StpUtil::getTokenName).thenReturn("satoken");
146+
stpUtil.when(StpUtil::getTokenValue).thenReturn("token-abc");
147+
148+
authService.login(new LoginRequest("alice", "Alice@123"));
149+
150+
// 关键:登录成功路径必须触发 lazy upgrade 写回
151+
verify(userAccountRepository).updatePasswordHash(1L, newBcryptHash);
152+
}
153+
}
154+
155+
/**
156+
* INV-003 lazy upgrade 容错:repository 写失败时不能阻断登录。
157+
*/
158+
@Test
159+
void loginStillSucceedsWhenLazyUpgradeWriteFails() {
160+
String legacyHash = "ad89b64d66caa8e30e5d5ce4a9763f4ecc205814c412175f3e2c50027471426d";
161+
UserAccount account = enabledUser(1L, "alice", legacyHash);
162+
when(userCenterService.findByUsername("alice")).thenReturn(Optional.of(account));
163+
when(passwordService.matches("Alice@123", legacyHash)).thenReturn(true);
164+
when(passwordService.isLegacyHash(legacyHash)).thenReturn(true);
165+
when(passwordService.hash("Alice@123")).thenReturn("$2b$10$newhash");
166+
// 模拟 DB 抖动:updatePasswordHash 抛异常
167+
org.mockito.Mockito.doThrow(new RuntimeException("simulated DB hiccup"))
168+
.when(userAccountRepository).updatePasswordHash(any(), any());
169+
170+
try (MockedStatic<StpUtil> stpUtil = mockStatic(StpUtil.class)) {
171+
stpUtil.when(StpUtil::getTokenName).thenReturn("satoken");
172+
stpUtil.when(StpUtil::getTokenValue).thenReturn("token-abc");
173+
174+
// 不阻断登录:仍返回有效 LoginResponse
175+
LoginResponse response = authService.login(new LoginRequest("alice", "Alice@123"));
176+
assertThat(response.tokenValue()).isEqualTo("token-abc");
177+
}
178+
}
179+
180+
/**
181+
* INV-003 反向:bcrypt 用户登录路径不应该触发 lazy upgrade(已经是 bcrypt 了)。
182+
*/
183+
@Test
184+
void loginDoesNotUpgradeWhenHashIsAlreadyBcrypt() {
185+
String bcryptHash = "$2b$10$alreadybcrypt$alreadybcrypt$alreadybcrypt$alreadybcryptxx";
186+
UserAccount account = enabledUser(1L, "alice", bcryptHash);
187+
when(userCenterService.findByUsername("alice")).thenReturn(Optional.of(account));
188+
when(passwordService.matches("Alice@123", bcryptHash)).thenReturn(true);
189+
when(passwordService.isLegacyHash(bcryptHash)).thenReturn(false);
190+
191+
try (MockedStatic<StpUtil> stpUtil = mockStatic(StpUtil.class)) {
192+
stpUtil.when(StpUtil::getTokenName).thenReturn("satoken");
193+
stpUtil.when(StpUtil::getTokenValue).thenReturn("token-abc");
194+
195+
authService.login(new LoginRequest("alice", "Alice@123"));
196+
197+
// 不应该调 hash() 也不应该调 updatePasswordHash()
198+
verify(passwordService, org.mockito.Mockito.never()).hash(any());
199+
verify(userAccountRepository, org.mockito.Mockito.never())
200+
.updatePasswordHash(any(), any());
201+
}
202+
}
203+
131204
// =============================================
132205
// loginByGithub() - GitHub OAuth 登录
133206
// =============================================

‎src/test/resources/test-schema.sql‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ CREATE TABLE IF NOT EXISTS user_follows (
5252
FOREIGN KEY (follower_id) REFERENCES user_accounts(id) ON DELETE CASCADE,
5353
FOREIGN KEY (followee_id) REFERENCES user_accounts(id) ON DELETE CASCADE
5454
);
55+
-- 与生产 schema 一致的"粉丝列表"二级索引;测试 schema 缺这个索引会让
56+
-- "索引被误删"这种 drift 逃过 CI(即便表本身存在)
57+
CREATE INDEX IF NOT EXISTS idx_user_follows_followee
58+
ON user_follows(followee_id, created_at DESC);
5559

5660
CREATE TABLE IF NOT EXISTS event_interests (
5761
event_id BIGINT NOT NULL,

0 commit comments

Comments
 (0)