Skip to content

Fix/docker demo stability - #24

Merged
meilingrl merged 13 commits into
masterfrom
fix/docker-demo-stability
Jun 4, 2026
Merged

meilingrl merged 13 commits into
masterfrom
fix/docker-demo-stability

Conversation

@meilingrl

Copy link
Copy Markdown
Owner

What changed

Type

  • feat — new feature
  • fix — bug fix
  • refactor — no behavior change
  • docs / task — documentation or task lifecycle only
  • chore — tooling, config, deps

Testing

  • cd backend && mvnw.cmd test — all pass
  • cd frontend && npm test — all pass
  • cd frontend && npm run build — no errors
  • Manual smoke: golden path works, no obvious regressions

Checklist

  • CHANGELOG.md prepended (skip for docs-only)
  • API spec updated if endpoint contract changed (docs/09-api-spec/)
  • .http smoke file updated if endpoint added or changed (docs/06-http/)
  • Task doc archived if this closes an active task (docs/08-tasks/)
  • No unrelated files staged

Notes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces launch-preparation updates, including production-like CORS validation, mock-auth gating, mock-payment guards, and optional Redis-backed caching for search and recommendations. It also adds privacy compliance features such as legal document pages, cookie consent, registration agreement validation, personal data export, and soft account closure, alongside an expanded demo dataset and staging infrastructure configurations. Feedback highlights three key areas for improvement: refactoring blocking Redis key scans to non-blocking cursors to prevent latency spikes, anonymizing user addresses during account deletion to ensure complete privacy compliance, and caching or removing the database probe in the unauthenticated health endpoint to mitigate Denial of Service risks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +85 to +98
public void evictByPrefix(String prefix) {
if (!isEnabled()) {
return;
}
String namespacedPrefix = namespacedKey(prefix);
try {
Set<String> keys = redisTemplate.keys(namespacedPrefix + "*");
if (keys != null && !keys.isEmpty()) {
redisTemplate.delete(keys);
}
} catch (RuntimeException ex) {
log.warn("Redis cache prefix eviction failed for prefix={}", namespacedPrefix, ex);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using redisTemplate.keys() is a blocking O(N) operation that scans the entire keyspace of the Redis database. In production or high-traffic environments, this can block the single-threaded Redis event loop, leading to latency spikes, timeouts, and potential service degradation. Since this method is triggered on frequent operations like order completion, product updates, and review submissions, it should be refactored to use a non-blocking SCAN cursor.

    public void evictByPrefix(String prefix) {
        if (!isEnabled()) {
            return;
        }
        String namespacedPrefix = namespacedKey(prefix);
        try {
            redisTemplate.execute((org.springframework.data.redis.connection.RedisConnection connection) -> {
                try (org.springframework.data.redis.core.Cursor<byte[]> cursor = connection.scan(
                        org.springframework.data.redis.core.ScanOptions.scanOptions()
                                .match(namespacedPrefix + "*")
                                .count(1000)
                                .build())) {
                    while (cursor.hasNext()) {
                        connection.keyCommands().del(cursor.next());
                    }
                } catch (Exception e) {
                    log.warn("Error during Redis scan eviction", e);
                }
                return null;
            });
        } catch (RuntimeException ex) {
            log.warn("Redis cache prefix eviction failed for prefix={}", namespacedPrefix, ex);
        }
    }

Comment on lines +618 to +632
@Override
public void anonymizeAndCloseAccount(Long userId, String anonymizedUsername, String passwordHash) {
jdbcTemplate.update(
"""
UPDATE users
SET username = ?, phone = NULL, email = NULL, password_hash = ?, nickname = ?,
avatar = NULL, status = 'deleted', updated_at = CURRENT_TIMESTAMP
WHERE id = ?
""",
anonymizedUsername,
passwordHash,
"Deleted account",
userId
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

When a user requests account deletion, their profile in the users table is anonymized, but their associated addresses in the user_addresses table are left completely intact. These addresses contain highly sensitive PII (receiver name, phone number, and detailed physical address). To ensure proper privacy compliance (e.g., GDPR/PII protection), these addresses must be anonymized or deleted during the account closure process.

    @Override
    public void anonymizeAndCloseAccount(Long userId, String anonymizedUsername, String passwordHash) {
        jdbcTemplate.update(
                """
                        UPDATE users
                        SET username = ?, phone = NULL, email = NULL, password_hash = ?, nickname = ?,
                            avatar = NULL, status = 'deleted', updated_at = CURRENT_TIMESTAMP
                        WHERE id = ?
                        """,
                anonymizedUsername,
                passwordHash,
                "Deleted account",
                userId
        );
        jdbcTemplate.update(
                """
                        UPDATE user_addresses
                        SET receiver_name = 'Deleted', receiver_phone = NULL, detail_address = 'Deleted'
                        WHERE user_id = ?
                        """,
                userId
        );
    }

Comment on lines +34 to +41
private String databaseStatus() {
try {
jdbcTemplate.queryForObject("SELECT 1", Integer.class);
return "UP";
} catch (RuntimeException ex) {
return "DOWN";
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

The /api/health endpoint is unauthenticated and publicly accessible. Executing a database query (SELECT 1) on every request makes it highly vulnerable to Denial of Service (DoS) attacks via database connection pool exhaustion. Consider caching the database health status for a short duration (e.g., 5-10 seconds) or removing the database probe from this public endpoint, relying instead on the secured /actuator/health endpoint for detailed infrastructure monitoring.

@meilingrl
meilingrl merged commit 4fb7a48 into master Jun 4, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant