fix(http): respond with 405 Method Not Allowed and harden static file symlinks

[AbhiPra24]

Pull Request Template

Description:

• Addressed Issue Wrong HTTP method on a registered route returns 404, not 405 #3853: When a request arrives for a registered route with an unsupported HTTP method, the router responds with 405 Method Not Allowed with the Allow header populated with accepted methods (e.g. Allow: GET, POST) and GoFr error JSON envelope {"error":{"message":"method not allowed"}}, rather than 404 Not Found.
• Configured NotFoundHandler and MethodNotAllowedHandler on the router and registered routes with Path before Methods so Gorilla Mux accurately tracks method mismatches.
• Added AllowedMethods(r *http.Request) []string to inspect allowed HTTP methods for the requested path.
• Addressed Issue Static file serving follows symlinks out of the served directory; replace lexical containment with os.Root #3855: Hardened static file serving by verifying filepath.EvalSymlinks remains within the root directory before opening/serving files. Symlinks pointing outside the served root return 404 Not Found.
• Added unit tests covering 405/404 handling, Allow header formatting, and symlink containment scenarios.

Fixes #3853
Fixes #3855

Breaking Changes (if applicable):

• None. Requests to existing routes with unsupported HTTP methods will now receive standard 405 Method Not Allowed instead of 404 Not Found.

Additional Information:

• All new code is covered by unit tests.
• Code is formatted with gofmt.

Checklist:

• I have formatted my code using goimport and golangci-lint.
• All new code is covered by unit tests.
• This PR does not decrease the overall code coverage.
• I have reviewed the code comments and documentation for clarity.

[aryanmehrotra]

Thanks @AbhiPra24 — I ran this against a real server rather than reading the diff, and both bugs are real and both of your fixes work. There is one regression that blocks it, and it is not visible from the diff or from the unit tests.

Both issues reproduce on development

#3853  POST /only-get        -> 404, no Allow header       (should be 405)
#3855  GET /static/evil.txt  -> 200, body "TOPSECRET"      (file outside the served root)

Your fixes do what they claim

probe	development	this PR
POST /only-get	404	405, Allow: GET
DELETE /ping	404	405, Allow: GET, POST
GET /static/evil.txt	200 TOPSECRET	404
GET /static/evildir/passwords.txt	200	404
dotdot-inside.txt (relative .. landing back inside)	200	200 — correctly still served

Same results under GOFR_ROUTER=trie, so the two matchers stay consistent. The Allow values are right, and the symlink containment does not over-reject.

---

Blocking: CORS preflight now returns 405

OPTIONS /only-get   Origin: http://x.test   Access-Control-Request-Method: GET

development:  HTTP/1.1 200 OK
              Access-Control-Allow-Methods: GET, POST, OPTIONS
              Access-Control-Allow-Origin: *

this PR:      HTTP/1.1 405 Method Not Allowed
              Allow: GET
              (no CORS headers at all)

Every cross-origin browser call to a GoFr app breaks — the preflight fails, so the browser never sends the real request.

Why, from the pinned gorilla/mux v1.8.1 (mux.go:138-168): the middleware chain is applied only in the matched-route branch, guarded by match.MatchErr == nil:

for _, route := range r.routes {
    if route.Match(req, match) {
        if match.MatchErr == nil {                       // <- middleware only here
            for i := len(r.middlewares) - 1; i >= 0; i-- {
                match.Handler = r.middlewares[i].Middleware(match.Handler)
            }
        }
        return true
    }
}

if match.MatchErr == ErrMethodMismatch {
    if r.MethodNotAllowedHandler != nil {
        match.Handler = r.MethodNotAllowedHandler        // <- no middleware
        return true
    }
    ...
}
if r.NotFoundHandler != nil {
    match.Handler = r.NotFoundHandler                    // <- no middleware
    ...
}

middleware.CORS is registered through router.Use(...) (pkg/gofr/http_server.go:67-70, alongside Tracer, Logging and Metrics), so anything served by those two handler fields bypasses all four.

Blocking: 404 and 405 disappear from logs and metrics

Same root cause, and it also hits 404 — which previously went through PathPrefix("/"), a real route, and therefore had the full chain. Measured on the same app:

	development	this PR
app_http_response_count{path="/nope",status="404"}	1	absent
app_http_response_count{path="/only-get"}	1	absent
access-log lines for /nope	3	0
access-log lines for /only-get	2	0

Only the /ping 200 is still counted. A scanner walking unknown paths, or a client using the wrong verb, becomes completely invisible to logs, metrics and traces.

A shape that keeps the middleware

Rather than moving to mux's handler fields, keep the catch-all as a real route and decide inside it:

a.httpServer.router.PathPrefix("/").Handler(handler{
    function:  catchAllHandler,   // 405 + Allow when AllowedMethods(r) is non-empty, else 404
    container: a.container,
})

AllowedMethods already skips routes with no methods, so the catch-all does not report itself. Because the catch-all matches, match.MatchErr stays nil, the whole chain runs, and CORS still intercepts OPTIONS before the handler is reached. Your AllowedMethods, ErrorMethodNotAllowed and the Allow formatting all carry over unchanged — this only changes where the decision is made.

If you go that way, worth re-checking whether the Path(...).Methods(...) reorder in Router.Add is still needed, since ErrMethodMismatch would no longer be what drives the 405.

---

Smaller things

An unrelated line was deleted. gofr.go drops:

if a.container.Logger != nil {
    a.container.Logger.Infof("Registered HTTP server on port: %d", a.httpServer.port)
}

Confirmed by running both: 1 such line on development, 0 here. It sits right where the catch-all was replaced, so it looks accidental.

Two new lint failures, both in the new test:

pkg/gofr/http/router_test.go:1007:21  G306: Expect WriteFile permissions to be 0600 or less (gosec)
pkg/gofr/http/router_test.go:1011:21  G306: (same)

Allow omits HEAD and OPTIONS. Allow: GET while OPTIONS /only-get is answered 405. RFC 9110 §10.2.1 has Allow list what the resource supports; fixing the CORS path will change this anyway, so it is worth settling both together.

On #3855, this departs from what the issue specified. The issue rules out this approach by name:

Adding filepath.EvalSymlinks before the comparison would close the common case but reintroduces the TOCTOU window os.Root exists to eliminate — the link can be repointed between the resolve and the open.

Being fair to your version: the residual window needs write access inside the served tree, which is a far higher bar than the original bug, and EvalSymlinks avoids the breaking change os.Root brings (an absolute symlink pointing inside the root keeps working, where os.Root would reject it). That may well be the better trade — but it is a deliberate divergence from the design the issue landed on, and it should be argued in the PR description rather than left implicit.

Two unrelated issues in one PR. #3853 is self-contained and, once the middleware regression is fixed, ready. #3855 has an open design question. Splitting them would let the 405 work merge without waiting on that discussion.

---

What I ran

check	result
go build ./...	clean
go test ./pkg/gofr/http/...	pass
go test ./pkg/gofr	fails identically on development (metrics-port FATAL) — not this PR
gofmt -l	2 dirty files, both pre-existing
golangci-lint --new-from-rev	2 new, listed above

The unit tests all pass, which is worth saying plainly: nothing in them exercises CORS, logging or metrics on a 404/405 path, which is why an end-to-end run was the only thing that surfaced it. A test asserting that a 404 still carries Access-Control-Allow-Origin would have caught it and would be worth adding alongside the fix.

Good work on both root causes — the symlink reproduction in particular is a real vulnerability and the containment logic you wrote handles the ..-back-inside case correctly, which is the part that is easy to get wrong.