🔒 (protect): Keep the bot check out of every cache
Changes
6 files changed, +63 -8
MODIFY
docs/admins/bot-protection.md
+4 -0
@@ -61,6 +61,10 @@
61
61
second challenge would only hand out a fresh budget, and challenging a caller who
62
62
cannot improve their standing loops forever.
63
63
64
+Refusals, challenge redirects and the check page itself are sent with
65
+`Cache-Control: no-store`, so a caching reverse proxy or CDN in front of the instance
66
+cannot hand one visitor's refusal — or one visitor's check page — to the next.
67
+
64
68
A solved check raises the budget; it does not remove it. A bypass would turn one
65
69
captcha solve — a few tenths of a cent at a solving farm — into a window of entirely
66
70
unmetered scraping, which is precisely the traffic the guard exists to stop.
MODIFY
docs/maintainers/bot-protection.md
+7 -0
@@ -90,6 +90,12 @@
90
90
`MessageDigest.isEqual`. Reusing the captcha secret rather than `GITSHARK_SECRET_KEY`
91
91
keeps the feature independent of the mirror/CI secret setup.
92
92
93
+**Nothing about the check is cacheable.** The `429`, the `303` towards `/challenge`
94
+and the check page all carry `Cache-Control: no-store`. Only the `200` page would have
95
+been at risk — `303` and `429` are not cacheable by default — but a response with no
96
+cache directive at all is heuristically cacheable, and a proxy holding the page across
97
+a provider switch or key rotation serves a widget that can no longer be solved.
98
+
93
99
**Fail closed on verification, fail open on configuration.** An unreachable or
94
100
unparseable `siteverify` reply is "not verified" — a broken provider must not become a
95
101
bypass. Conversely a missing/incomplete captcha config does not disable metering: it
@@ -111,6 +117,7 @@
111
117
- Signed, self-expiring pass cookie that moves its holder to the user budget for its
112
118
lifetime, metered under its own key.
113
119
- Open-redirect-safe `?redirect=` handling (server-relative single-slash paths only).
120
+- `Cache-Control: no-store` on every refusal, redirect and check-page response.
114
121
- Client IP taken from the proxy-aware remote address.
115
122
116
123
## What still needs to be implemented
MODIFY
src/main/java/de/workaround/protect/ChallengeResource.java
+16 -6
@@ -13,6 +13,7 @@
13
13
import jakarta.ws.rs.Produces;
14
14
import jakarta.ws.rs.QueryParam;
15
15
import jakarta.ws.rs.core.Context;
16
+import jakarta.ws.rs.core.HttpHeaders;
16
17
import jakarta.ws.rs.core.MediaType;
17
18
import jakarta.ws.rs.core.MultivaluedMap;
18
19
import jakarta.ws.rs.core.NewCookie;
@@ -51,10 +52,10 @@
51
52
52
53
@GET
53
54
@Produces(MediaType.TEXT_HTML)
54
- public TemplateInstance page(@QueryParam("redirect") String redirect)
55
+ public Response page(@QueryParam("redirect") String redirect)
55
56
{
56
57
requireCaptcha();
57
- return render(safeRedirect(redirect), null);
58
+ return uncached(Response.ok(render(safeRedirect(redirect), null)));
58
59
}
59
60
60
61
@POST
@@ -67,9 +68,8 @@
67
68
String token = form.getFirst(config.provider().responseField());
68
69
if (!verifier.verify(token, clientAddress.ip()))
69
70
{
70
- return Response.status(Response.Status.FORBIDDEN)
71
- .entity(render(redirect, "That check did not go through. Please try again."))
72
- .build();
71
+ return uncached(Response.status(Response.Status.FORBIDDEN)
72
+ .entity(render(redirect, "That check did not go through. Please try again.")));
73
73
}
74
74
NewCookie pass = new NewCookie.Builder(HumanPass.COOKIE_NAME)
75
75
.value(humanPass.issue())
@@ -79,7 +79,17 @@
79
79
.sameSite(NewCookie.SameSite.LAX)
80
80
.secure("https".equalsIgnoreCase(uriInfo.getRequestUri().getScheme()))
81
81
.build();
82
- return Response.seeOther(URI.create(redirect)).cookie(pass).build();
82
+ return uncached(Response.seeOther(URI.create(redirect)).cookie(pass));
83
+ }
84
+
85
+ /**
86
+ * The check is per-visitor and short-lived — the widget's own token certainly is — so no proxy or
87
+ * browser may keep any of it. Without this the page carries no cache directive at all and is
88
+ * heuristically cacheable.
89
+ */
90
+ private static Response uncached(Response.ResponseBuilder builder)
91
+ {
92
+ return builder.header(HttpHeaders.CACHE_CONTROL, ExpensiveRequestFilter.NO_STORE).build();
83
93
}
84
94
85
95
private TemplateInstance render(String redirect, String error)
MODIFY
src/main/java/de/workaround/protect/ExpensiveRequestFilter.java
+11 -1
@@ -9,6 +9,7 @@
9
9
import jakarta.ws.rs.container.ContainerRequestContext;
10
10
import jakarta.ws.rs.container.ContainerRequestFilter;
11
11
import jakarta.ws.rs.core.Cookie;
12
+import jakarta.ws.rs.core.HttpHeaders;
12
13
import jakarta.ws.rs.core.MediaType;
13
14
import jakarta.ws.rs.core.Response;
14
15
import jakarta.ws.rs.ext.Provider;
@@ -35,6 +36,12 @@
35
36
@Provider
36
37
public class ExpensiveRequestFilter implements ContainerRequestFilter
37
38
{
39
+ /**
40
+ * Refusals and challenge redirects are per-caller state, never shared content: without this a
41
+ * proxy in front of the instance may hand one visitor's answer to the next.
42
+ */
43
+ static final String NO_STORE = "no-store";
44
+
38
45
@Inject
39
46
ProtectionConfig config;
40
47
@@ -102,13 +109,16 @@
102
109
target = target + "?" + query;
103
110
}
104
111
String location = "/challenge?redirect=" + URLEncoder.encode(target, StandardCharsets.UTF_8);
105
- return Response.seeOther(URI.create(location)).build();
112
+ return Response.seeOther(URI.create(location))
113
+ .header(HttpHeaders.CACHE_CONTROL, NO_STORE)
114
+ .build();
106
115
}
107
116
108
117
private Response refusal()
109
118
{
110
119
return Response.status(Response.Status.TOO_MANY_REQUESTS)
111
120
.header("Retry-After", Math.max(1, config.window().toSeconds()))
121
+ .header(HttpHeaders.CACHE_CONTROL, NO_STORE)
112
122
.type(MediaType.TEXT_PLAIN)
113
123
.entity("Too many expensive requests. Please slow down and try again shortly.\n")
114
124
.build();
MODIFY
src/test/java/de/workaround/protect/ChallengeFlowTest.java
+22 -0
@@ -104,6 +104,7 @@
104
104
.then().statusCode(303)
105
105
// JAX-RS absolutizes Location; only the target path is interesting here
106
106
.header("Location", endsWith(commit))
107
+ .header("Cache-Control", containsString("no-store"))
107
108
.extract().response();
108
109
109
110
String pass = solved.getCookie("gitshark_human");
@@ -144,6 +145,27 @@
144
145
.header("Retry-After", notNullValue());
145
146
}
146
147
148
+ @Test
149
+ void nothingAboutTheCheckIsCacheable()
150
+ {
151
+ String commit = seedCommitPath("ch-nocache");
152
+ given().when().get(commit).then().statusCode(200);
153
+
154
+ // the 303 towards the check, the check page itself, and a rejected attempt
155
+ given().redirects().follow(false).when().get(commit)
156
+ .then().statusCode(303)
157
+ .header("Cache-Control", containsString("no-store"));
158
+ given().when().get("/challenge?redirect=" + commit)
159
+ .then().statusCode(200)
160
+ .header("Cache-Control", containsString("no-store"));
161
+ given().redirects().follow(false)
162
+ .formParam("redirect", commit)
163
+ .formParam("cf-turnstile-response", "bad-token")
164
+ .when().post("/challenge")
165
+ .then().statusCode(403)
166
+ .header("Cache-Control", containsString("no-store"));
167
+ }
168
+
147
169
private String solve(String redirect)
148
170
{
149
171
return given().redirects().follow(false)
MODIFY
src/test/java/de/workaround/protect/ExpensiveRequestLimitTest.java
+3 -1
@@ -20,6 +20,7 @@
20
20
import jakarta.transaction.Transactional;
21
21
22
22
import static io.restassured.RestAssured.given;
23
+import static org.hamcrest.CoreMatchers.containsString;
23
24
import static org.hamcrest.CoreMatchers.notNullValue;
24
25
25
26
/**
@@ -72,7 +73,8 @@
72
73
given().when().get(commit).then().statusCode(200);
73
74
given().when().get(commit)
74
75
.then().statusCode(429)
75
- .header("Retry-After", notNullValue());
76
+ .header("Retry-After", notNullValue())
77
+ .header("Cache-Control", containsString("no-store"));
76
78
}
77
79
78
80
@Test