Skip to content

Commit cdb7563

Browse files
f0sselclaude
andcommitted
Remove Action type from rules package, simplify to boolean logic
- Remove Action enum (Allow/Deny) from rules package - Update rules engine to return bool instead of Action - Update EvaluationResult to use Allowed bool field - Update audit package to use boolean logic instead of Action - Update proxy to use boolean conditions (!result.Allowed) - Update all tests to use true/false instead of Allow/Deny - Remove unnecessary dependencies between packages Simplifies codebase by using intuitive boolean logic throughout 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent d4178ac commit cdb7563

File tree

5 files changed

+45
-66
lines changed

5 files changed

+45
-66
lines changed

audit/audit.go

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,15 @@ package audit
33
import (
44
"log/slog"
55
"net/http"
6-
7-
"github.com/coder/jail/rules"
86
)
97

108
// Request represents information about an HTTP request for auditing
119
type Request struct {
12-
Method string
13-
URL string
14-
Action rules.Action
15-
Rule string // The rule that matched (if any)
16-
Reason string // Reason for the action (e.g., "no matching allow rules")
10+
Method string
11+
URL string
12+
Allowed bool
13+
Rule string // The rule that matched (if any)
14+
Reason string // Reason for the action (e.g., "no matching allow rules")
1715
}
1816

1917
// Auditor handles audit logging for HTTP requests
@@ -36,13 +34,12 @@ func NewLoggingAuditor(logger *slog.Logger) *LoggingAuditor {
3634

3735
// AuditRequest logs the request using structured logging
3836
func (a *LoggingAuditor) AuditRequest(req *Request) {
39-
switch req.Action {
40-
case rules.Allow:
37+
if req.Allowed {
4138
a.logger.Info("ALLOW",
4239
"method", req.Method,
4340
"url", req.URL,
4441
"rule", req.Rule)
45-
case rules.Deny:
42+
} else {
4643
a.logger.Warn("DENY",
4744
"method", req.Method,
4845
"url", req.URL,

audit/audit_test.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import (
44
"log/slog"
55
"net/http"
66
"testing"
7-
8-
"github.com/coder/jail/rules"
97
)
108

119
func TestLoggingAuditor(t *testing.T) {
@@ -23,19 +21,19 @@ func TestLoggingAuditor(t *testing.T) {
2321
{
2422
name: "allow request",
2523
request: &Request{
26-
Method: "GET",
27-
URL: "https://github.com",
28-
Action: rules.Allow,
29-
Rule: "allow github.com",
24+
Method: "GET",
25+
URL: "https://github.com",
26+
Allowed: true,
27+
Rule: "allow github.com",
3028
},
3129
},
3230
{
3331
name: "deny request",
3432
request: &Request{
35-
Method: "POST",
36-
URL: "https://example.com",
37-
Action: rules.Deny,
38-
Reason: "no matching allow rules",
33+
Method: "POST",
34+
URL: "https://example.com",
35+
Allowed: false,
36+
Reason: "no matching allow rules",
3937
},
4038
},
4139
}

proxy/proxy.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,11 @@ func (p *ProxyServer) handleHTTP(w http.ResponseWriter, r *http.Request) {
110110

111111
// Audit the request
112112
auditReq := audit.HTTPRequestToAuditRequest(r)
113-
auditReq.Action = result.Action
113+
auditReq.Allowed = result.Allowed
114114
auditReq.Rule = result.Rule
115115
p.auditRequest(auditReq)
116116

117-
if result.Action == rules.Deny {
117+
if !result.Allowed {
118118
p.writeBlockedResponse(w, r)
119119
return
120120
}
@@ -136,14 +136,14 @@ func (p *ProxyServer) handleHTTPS(w http.ResponseWriter, r *http.Request) {
136136

137137
// Audit the request
138138
auditReq := &audit.Request{
139-
Method: r.Method,
140-
URL: fullURL,
141-
Action: result.Action,
142-
Rule: result.Rule,
139+
Method: r.Method,
140+
URL: fullURL,
141+
Allowed: result.Allowed,
142+
Rule: result.Rule,
143143
}
144144
p.auditRequest(auditReq)
145145

146-
if result.Action == rules.Deny {
146+
if !result.Allowed {
147147
p.writeBlockedResponse(w, r)
148148
return
149149
}
@@ -292,7 +292,7 @@ For more help: https://github.com/coder/jail
292292

293293
// auditRequest handles auditing of requests
294294
func (p *ProxyServer) auditRequest(req *audit.Request) {
295-
if req.Action == rules.Deny {
295+
if !req.Allowed {
296296
req.Reason = "no matching allow rules"
297297
}
298298
p.auditor.AuditRequest(req)

rules/rules.go

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,6 @@ import (
66
"strings"
77
)
88

9-
// Action represents whether to allow a request
10-
type Action int
11-
12-
const (
13-
Allow Action = iota
14-
Deny // Default deny when no allow rules match
15-
)
16-
17-
func (a Action) String() string {
18-
switch a {
19-
case Allow:
20-
return "ALLOW"
21-
default:
22-
return "DENY"
23-
}
24-
}
259

2610
// Rule represents an allow rule with optional HTTP method restrictions
2711
type Rule struct {
@@ -139,39 +123,39 @@ func NewRuleEngine(rules []*Rule, logger *slog.Logger) *RuleEngine {
139123

140124
// EvaluationResult contains the result of rule evaluation
141125
type EvaluationResult struct {
142-
Action Action
143-
Rule string // The rule that matched (if any)
126+
Allowed bool
127+
Rule string // The rule that matched (if any)
144128
}
145129

146-
// Evaluate evaluates a request against all allow rules and returns the action to take
147-
func (re *RuleEngine) Evaluate(method, url string) Action {
130+
// Evaluate evaluates a request against all allow rules and returns true if allowed
131+
func (re *RuleEngine) Evaluate(method, url string) bool {
148132
// Check if any allow rule matches
149133
for _, rule := range re.rules {
150134
if rule.Matches(method, url) {
151-
return Allow
135+
return true
152136
}
153137
}
154138

155139
// Default deny if no allow rules match
156-
return Deny
140+
return false
157141
}
158142

159-
// EvaluateWithRule evaluates a request and returns both action and matching rule
143+
// EvaluateWithRule evaluates a request and returns both result and matching rule
160144
func (re *RuleEngine) EvaluateWithRule(method, url string) EvaluationResult {
161145
// Check if any allow rule matches
162146
for _, rule := range re.rules {
163147
if rule.Matches(method, url) {
164148
return EvaluationResult{
165-
Action: Allow,
166-
Rule: rule.Raw,
149+
Allowed: true,
150+
Rule: rule.Raw,
167151
}
168152
}
169153
}
170154

171155
// Default deny if no allow rules match
172156
return EvaluationResult{
173-
Action: Deny,
174-
Rule: "",
157+
Allowed: false,
158+
Rule: "",
175159
}
176160
}
177161

rules/rules_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -240,12 +240,12 @@ func TestRuleEngine(t *testing.T) {
240240
name string
241241
method string
242242
url string
243-
expected Action
243+
expected bool
244244
}{
245-
{"allow github", "GET", "https://github.com/user/repo", Allow},
246-
{"allow api GET", "GET", "https://api.example.com", Allow},
247-
{"deny api POST", "POST", "https://api.example.com", Deny},
248-
{"deny other", "GET", "https://example.com", Deny},
245+
{"allow github", "GET", "https://github.com/user/repo", true},
246+
{"allow api GET", "GET", "https://api.example.com", true},
247+
{"deny api POST", "POST", "https://api.example.com", false},
248+
{"deny other", "GET", "https://example.com", false},
249249
}
250250

251251
for _, tt := range tests {
@@ -275,13 +275,13 @@ func TestRuleEngineWildcardRules(t *testing.T) {
275275
name string
276276
method string
277277
url string
278-
expected Action
278+
expected bool
279279
}{
280-
{"allow github", "GET", "https://github.com", Allow},
281-
{"allow github subdomain", "POST", "https://github.io", Allow},
282-
{"allow api GET", "GET", "https://api.example.com", Allow},
283-
{"deny api POST", "POST", "https://api.example.com", Deny},
284-
{"deny unmatched", "GET", "https://example.org", Deny},
280+
{"allow github", "GET", "https://github.com", true},
281+
{"allow github subdomain", "POST", "https://github.io", true},
282+
{"allow api GET", "GET", "https://api.example.com", true},
283+
{"deny api POST", "POST", "https://api.example.com", false},
284+
{"deny unmatched", "GET", "https://example.org", false},
285285
}
286286

287287
for _, tt := range tests {

0 commit comments

Comments
 (0)