Skip to content

Commit d06b440

Browse files
security: harden captcha integration after peer review
- Reject null/empty token server-side before making outbound hCaptcha call - Add LogCaptchaValidationFailed [LoggerMessage] for observability on rejections - Document fail-open policy with explicit rationale comment - JS: add captchaPending flag to block concurrent getCaptchaToken() calls - JS: wrap getCaptchaToken() in 15-second timeout race to prevent UI freeze - JS: clearTimeout(timeoutId) in resolve/reject wrappers to prevent stale timer from corrupting state of subsequent calls
1 parent 914fb94 commit d06b440

2 files changed

Lines changed: 49 additions & 5 deletions

File tree

EssentialCSharp.Web/Controllers/ChatController.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,28 @@ public ChatController(ILogger<ChatController> logger, AIChatService aiChatServic
3737
/// <summary>
3838
/// Validates the hCaptcha token when captcha is configured.
3939
/// Returns <c>true</c> when captcha is not configured (dev mode) or when the token is valid.
40-
/// Fails open on hCaptcha service outages to avoid blocking legitimate users.
40+
/// Fails open on hCaptcha service outages (null result) to avoid blocking legitimate users —
41+
/// this is intentional given the existing [Authorize] + rate-limiting backstop.
4142
/// </summary>
4243
private async Task<bool> IsCaptchaValidAsync(string? token, string? remoteIp, CancellationToken ct)
4344
{
4445
if (string.IsNullOrWhiteSpace(_CaptchaOptions.SecretKey))
4546
return true; // captcha not configured — skip validation
4647

48+
if (string.IsNullOrWhiteSpace(token))
49+
return false; // token required when captcha is configured — reject without an outbound call
50+
4751
HCaptchaResult? result = await _CaptchaService.VerifyAsync(token, remoteIp, ct);
4852
if (result is null)
4953
{
50-
LogCaptchaServiceUnavailable(_Logger); // hCaptcha unreachable — fail open
54+
LogCaptchaServiceUnavailable(_Logger); // hCaptcha unreachable — fail open (intentional)
5155
return true;
5256
}
57+
58+
if (!result.Success)
59+
{
60+
LogCaptchaValidationFailed(_Logger, string.Join(',', result.ErrorCodes ?? []));
61+
}
5362
return result.Success;
5463
}
5564

@@ -224,6 +233,9 @@ public async Task StreamMessage([FromBody] ChatMessageRequest request, Cancellat
224233
[LoggerMessage(Level = LogLevel.Warning, Message = "hCaptcha service unavailable during chat request — failing open")]
225234
private static partial void LogCaptchaServiceUnavailable(ILogger<ChatController> logger);
226235

236+
[LoggerMessage(Level = LogLevel.Warning, Message = "hCaptcha validation failed for chat request — error codes: {ErrorCodes}")]
237+
private static partial void LogCaptchaValidationFailed(ILogger<ChatController> logger, string errorCodes);
238+
227239
[LoggerMessage(Level = LogLevel.Debug, Message = "Chat stream cancelled for user {User}")]
228240
private static partial void LogChatStreamCancelled(ILogger<ChatController> logger, string? user);
229241

EssentialCSharp.Web/wwwroot/js/chat-module.js

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ const captchaSiteKey = window.HCAPTCHA_SITE_KEY || null;
1818
let captchaWidgetId = null;
1919
let captchaTokenResolve = null;
2020
let captchaTokenReject = null;
21+
let captchaPending = false; // prevents concurrent token requests overwriting promise callbacks
22+
const CAPTCHA_TIMEOUT_MS = 15_000;
2123

2224
function initCaptchaWidget() {
2325
if (!captchaSiteKey) return;
@@ -56,17 +58,47 @@ function initCaptchaWidget() {
5658
/**
5759
* Returns a fresh hCaptcha token, or null if captcha is not configured.
5860
* Resolves after the invisible challenge completes (typically instant for non-suspicious users).
61+
* Rejects with 'captcha-concurrent' if a token request is already in-flight.
62+
* Rejects with 'captcha-timeout' if the widget does not respond within 15 seconds.
5963
*/
6064
function getCaptchaToken() {
6165
if (!captchaSiteKey || captchaWidgetId === null) return Promise.resolve(null);
62-
return new Promise((resolve, reject) => {
63-
captchaTokenResolve = resolve;
64-
captchaTokenReject = reject;
66+
if (captchaPending) return Promise.reject(new Error('captcha-concurrent'));
67+
captchaPending = true;
68+
69+
let timeoutId;
70+
71+
const tokenPromise = new Promise((resolve, reject) => {
72+
captchaTokenResolve = (token) => {
73+
captchaPending = false;
74+
clearTimeout(timeoutId); // cancel lingering timer so it can't corrupt the next call
75+
resolve(token);
76+
};
77+
captchaTokenReject = (err) => {
78+
captchaPending = false;
79+
clearTimeout(timeoutId);
80+
reject(err);
81+
};
6582
window.hcaptcha.execute(captchaWidgetId);
6683
});
84+
85+
const timeoutPromise = new Promise((_, reject) => {
86+
// timeoutId is assigned synchronously here, before captchaTokenResolve can fire
87+
timeoutId = setTimeout(() => {
88+
captchaPending = false;
89+
captchaTokenResolve = null;
90+
captchaTokenReject = null;
91+
reject(new Error('captcha-timeout'));
92+
}, CAPTCHA_TIMEOUT_MS);
93+
});
94+
95+
return Promise.race([tokenPromise, timeoutPromise]);
6796
}
6897

6998
function resetCaptchaWidget() {
99+
captchaPending = false;
100+
captchaTokenResolve = null;
101+
captchaTokenReject = null;
70102
if (captchaWidgetId !== null && typeof window.hcaptcha?.reset === 'function') {
71103
window.hcaptcha.reset(captchaWidgetId);
72104
}

0 commit comments

Comments
 (0)