Commit f652e75
authored
feat(server): auto-generate auth token and document security risks (#803)
* feat(server): auto-generate auth token and document security risks
Implements two critical security improvements for gptme-server:
**C-1: Auto-generate Authentication Token**
- Modified get_server_token() to auto-generate secure token if GPTME_SERVER_TOKEN not set
- Added prominent WARNING log with generated token and setup instructions
- Fixes critical vulnerability: 'Authentication Disabled by Default'
- Token persists for server session lifetime
**C-2: Document Query Parameter Token Exposure**
- Added comprehensive security warnings in require_auth() docstring
- Documents exposure risks: server logs, browser history, referrer headers, proxy logs
- Added inline warning comments at query parameter fallback code
- Added debug logging when query param auth is used
- Recommends cookie-based auth for SSE as future improvement
These changes address findings from security audit (Session 427) and implement
immediate actions from the security checklist.
Related: Security audit at knowledge/infrastructure/gptme-server-security-audit.md
Session: 429 - Domain 3 Component 3
Co-authored-by: Bob <bob@superuserlabs.org>
* fix(server): make authentication conditional on bind address
- No auth required for local-only binding (127.0.0.1, localhost)
- Auth required when binding to network interfaces (0.0.0.0, specific IPs)
- Fixes all 21 test failures where tests expected no auth for local development
- Add init_auth() call in create_app() to initialize auth state for tests
- Update cli.py to pass host parameter to init_auth()
This addresses @ErikBjare's feedback that authentication should 'just work'
for local development without requiring tokens, while still requiring auth
when exposing the server to the network.
Co-authored-by: Bob <bob@superuserlabs.org>
* fix(server): add guard for server_token None case
Fixes mypy typecheck error at line 156 where compare_digest received
str | None instead of str. Added explicit check and error handling
for the case where server_token is None when auth is enabled.
* fix(tests): enable auth in test fixtures for network binding simulation
Tests were failing because:
- init_auth() wasn't called, so _auth_enabled stayed False
- @require_auth decorator skips checks when auth disabled
- Tests got 200 instead of expected 401
Fix:
- Call init_auth('0.0.0.0') in auth_token fixture to enable auth
- Restore original _auth_enabled state in cleanup
- Simulates network binding where auth is required
Co-authored-by: Bob <bob@superuserlabs.org>
* fix(server): pass actual host to create_app for correct auth behavior
- Modified create_app() to accept host parameter (defaults to 127.0.0.1)
- Pass actual bind address from cli serve command
- Fixes issue where auth was always disabled due to hardcoded localhost
Addresses feedback from @ErikBjare in PR #803:
Auth should be disabled for localhost binding (127.0.0.1) but enabled
for network binding (0.0.0.0), based on actual server bind address.1 parent abdb449 commit f652e75
4 files changed
+118
-35
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
468 | 468 | | |
469 | 469 | | |
470 | 470 | | |
471 | | - | |
| 471 | + | |
472 | 472 | | |
473 | 473 | | |
474 | 474 | | |
| |||
506 | 506 | | |
507 | 507 | | |
508 | 508 | | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
509 | 514 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
| |||
16 | 17 | | |
17 | 18 | | |
18 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
19 | 23 | | |
20 | 24 | | |
21 | 25 | | |
| |||
26 | 30 | | |
27 | 31 | | |
28 | 32 | | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
29 | 45 | | |
30 | 46 | | |
31 | 47 | | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
32 | 51 | | |
33 | 52 | | |
34 | | - | |
| 53 | + | |
35 | 54 | | |
36 | 55 | | |
37 | 56 | | |
| |||
40 | 59 | | |
41 | 60 | | |
42 | 61 | | |
43 | | - | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
44 | 79 | | |
45 | 80 | | |
46 | 81 | | |
| |||
58 | 93 | | |
59 | 94 | | |
60 | 95 | | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
66 | 112 | | |
67 | 113 | | |
68 | 114 | | |
69 | 115 | | |
70 | 116 | | |
71 | | - | |
| 117 | + | |
| 118 | + | |
72 | 119 | | |
73 | 120 | | |
74 | 121 | | |
75 | 122 | | |
76 | | - | |
77 | | - | |
78 | | - | |
| 123 | + | |
| 124 | + | |
79 | 125 | | |
80 | 126 | | |
81 | | - | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
82 | 133 | | |
83 | 134 | | |
84 | 135 | | |
| |||
93 | 144 | | |
94 | 145 | | |
95 | 146 | | |
96 | | - | |
97 | | - | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
98 | 151 | | |
| 152 | + | |
| 153 | + | |
99 | 154 | | |
100 | 155 | | |
101 | 156 | | |
| |||
110 | 165 | | |
111 | 166 | | |
112 | 167 | | |
113 | | - | |
| 168 | + | |
114 | 169 | | |
115 | 170 | | |
116 | 171 | | |
| 172 | + | |
117 | 173 | | |
118 | 174 | | |
119 | 175 | | |
120 | | - | |
| 176 | + | |
| 177 | + | |
121 | 178 | | |
122 | | - | |
| 179 | + | |
123 | 180 | | |
124 | | - | |
125 | | - | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
126 | 185 | | |
127 | | - | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
128 | 193 | | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
129 | 207 | | |
130 | 208 | | |
131 | | - | |
| 209 | + | |
132 | 210 | | |
133 | | - | |
134 | | - | |
135 | 211 | | |
136 | | - | |
137 | | - | |
138 | | - | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | | - | |
143 | | - | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
144 | 217 | | |
145 | 218 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
85 | 85 | | |
86 | 86 | | |
87 | 87 | | |
88 | | - | |
| 88 | + | |
89 | 89 | | |
90 | | - | |
| 90 | + | |
91 | 91 | | |
92 | 92 | | |
93 | 93 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
21 | | - | |
| 21 | + | |
22 | 22 | | |
| 23 | + | |
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
28 | 32 | | |
29 | 33 | | |
30 | 34 | | |
31 | 35 | | |
32 | 36 | | |
| 37 | + | |
33 | 38 | | |
34 | 39 | | |
35 | 40 | | |
| |||
0 commit comments