nuts-foundation / nuts-foundation/nuts-node
discovery/api/server is the only strict-handler wrapper without audit.StrictMiddleware
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 28
- Forks
- 23
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 76
Description
Summary
discovery/api/server is the only generated strict-handler API wrapper that does not install audit.StrictMiddleware. Nothing is broken today, but the failure mode if an audit-emitting operation is ever added to that path is a panic rather than a missing log line, so the inconsistency is worth closing.
Where
discovery/api/server/api.go:55-67 registers a single StrictMiddlewareFunc, which sets the context keys and stops:
func (w *Wrapper) Routes(router core.EchoRouter) {
RegisterHandlers(router, NewStrictHandler(w, []StrictMiddlewareFunc{
func(f StrictHandlerFunc, operationID string) StrictHandlerFunc {
return func(ctx echo.Context, request interface{}) (response interface{}, err error) {
ctx.Set(core.OperationIDContextKey, operationID)
ctx.Set(core.ModuleNameContextKey, discovery.ModuleName)
ctx.Set(core.StatusCodeResolverContextKey, w)
return f(ctx, request)
}
},
}))
}
Compare vcr/api/vcr/v2/api.go:63-77, which registers a second function calling audit.StrictMiddleware(f, vcr.ModuleName, operationID). The same pattern is present in crypto/api/v1, didman/api/v1, auth/api/auth/v1, discovery/api/v1, vcr/api/openid4vci/v0 and vdr/api/v2.
(auth/api/means/v1/api.go:32-34 also has no audit middleware, but that wrapper only delegates via w.Auth.ContractNotary().Routes(router) and registers no handlers of its own, so it is not comparable.)
Why this is currently harmless
audit.StrictMiddleware (audit/http.go:28-34) does not log anything. It calls SetOnEchoContext, which puts actor, moduleName and operationID into the request context so that later explicit audit calls can use them.
Audit entries come from nine audit.Log(...) call sites: JWE encrypt and decrypt, JWT and JWS signing, key create and delete (crypto/), credential removed (vcr/holder/sql_wallet.go:151), credential retrieved (vcr/issuer/openid.go:271) and access granted or denied (http/tokenV2/middleware.go:444).
None of those is reachable from the public discovery endpoints. GET and POST /discovery/:serviceID verify and store presentations; they sign nothing and do not touch the wallet or the key store. So no audit entry is currently being lost, and requests are still recorded by requestLoggerMiddleware, which applies globally and skips only /metrics, /status and /health.
Why it is still worth fixing
audit.Log panics when the context has no audit info (audit/audit.go:180-182):
info := InfoFromContext(ctx)
if info == nil {
panic("audit: no audit info in context")
}
So the moment anyone adds an operation to a discovery server handler that emits an audit event, the request panics instead of logging.
That panic is not converted into a 500. Echo does not recover: Echo.ServeHTTP (echo.go:667-692 in labstack/echo v4.15.4) contains no recover(), and there is no middleware.Recover() anywhere in this repository. The panic therefore reaches Go's net/http per-connection recover in conn.serve, which logs http: panic serving ... with a stack trace and closes the connection. The caller sees a dropped connection rather than an HTTP error response.
This is a defensible fail-loud design, but it means a forgotten middleware surfaces as an unexplained connection reset plus a stack trace, which is a poor thing to debug under pressure.
Fix
Add the second StrictMiddlewareFunc to discovery/api/server/api.go, matching the other wrappers:
func(f StrictHandlerFunc, operationID string) StrictHandlerFunc {
return audit.StrictMiddleware(f, discovery.ModuleName, operationID)
},
Worth considering separately: whether registering a presentation on a discovery service should emit an audit event at all. There is no event constant for it today (audit/audit.go:34-57), and it is a write to node state performed by a third party, so it is a reasonable candidate for the audit trail.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with discovery/api/server/api.go:55-67 and compare its StrictMiddlewareFunc registrations with vcr/api/vcr/v2/api.go:63-77. Read audit/http.go:28-34 to confirm the middleware contract; done means the discovery wrapper also installs audit.StrictMiddleware with discovery.ModuleName and the operation ID.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100