docs(pkg): apply the comment rule to envs, wsconnadapter and jwttoken

A worked sample of what the rule does to code that already exists, so the
policy can be judged on a diff rather than on its own description.

The godoc the linter asks for gets written — 22 exported symbols across
envs and wsconnadapter — and a caller's question that was answered by a
comment buried in the body moves up into the contract: why Read hides
io.EOF, why a failed ping write tears the adapter down, why Read holds a
mutex at all.

Everything else goes, and this is where the cost is legible. jwttoken's
encoder.go ends with no comments at all: encodeClaims, decodeClaims and
evalClaims are unexported, so no linter asks for their documentation and
the rule does not write it. The same removes the doc on claimKind,
userClaims, deviceClaims, enrollmentDecisionClaims and claimKindFromString,
and the note on wsconnadapter's pingInterval and pongTimeout fields saying
tests are the only thing that overrides them. jwttoken's TODO list leaves
the source as well — a rename nobody is doing today is an issue.

The caveat this adapter exists for is gorilla/websocket#441: a websocket
connection is message-oriented and the net.Conn it stands in for is not, so
reads are stitched across frames and writes are serialised. That is the
whole reason for the type, and under this rule the commit message is the
only place it can be recorded. It is recorded here.

Tests pass and golangci-lint is clean on all three packages with the
exported rule on.

Author: otavio

pkg/api/jwttoken/claims.go

@@ -12,34 +12,21 @@ import (
 	"github.com/shellhub-io/shellhub/pkg/uuid"
 )
 
-// TODO:
-// 1. Rename [user|device]Claims.Kind JSON's tag to "kind". (BREAKING CHANGE)
-// 2. Rename this package to jwt.
-
 type (
-	// claimKind represents the type of claims used in JWT tokens.
 	claimKind string
 
-	// userClaims is an auxiliary type that embeds [github.com/golang-jwt/jwt/v5.RegisteredClaims]
-	// into [github.com/shellhub-io/shellhub/pkg/api/authorizer.UserClaims] to convert it into
-	// [github.com/golang-jwt/jwt/v5.Claims] for use in an [encode] call.
 	userClaims struct {
 		Kind claimKind `json:"claims"`
 		authorizer.UserClaims
 		jwt.RegisteredClaims
 	}
 
-	// deviceClaims is an auxiliary type that embeds [github.com/golang-jwt/jwt/v5.RegisteredClaims]
-	// into [github.com/shellhub-io/shellhub/pkg/api/authorizer.DeviceClaims] to convert it into
-	// [github.com/golang-jwt/jwt/v5.Claims] for use in an [encode] call.
 	deviceClaims struct {
 		Kind claimKind `json:"claims"`
 		authorizer.DeviceClaims
 		jwt.RegisteredClaims
 	}
 
-	// enrollmentDecisionClaims is the auxiliary type used to sign a deferred enrollment-decision
-	// callback token.
 	enrollmentDecisionClaims struct {
 		Kind claimKind `json:"claims"`
 		EnrollmentDecisionClaims
@@ -62,7 +49,6 @@ const (
 	kindUnknownClaims            claimKind = "unknown"
 )
 
-// claimKindFromString converts a string to a claimKind.
 func claimKindFromString(str string) claimKind {
 	switch str {
 	case "user":

pkg/api/jwttoken/encoder.go

@@ -7,21 +7,16 @@ import (
 	"github.com/golang-jwt/jwt/v5"
 )
 
-// encodeClaims encodes the provided claims into a JWT token using the provided RSA private key.
-// It returns the encoded JWT token as a string and any error encountered during the encoding process.
-// The claims are signed using the RS256 signing method.
 func encodeClaims(claims jwt.Claims, privateKey *rsa.PrivateKey) (string, error) {
 	return jwt.NewWithClaims(jwt.SigningMethodRS256, claims).SignedString(privateKey)
 }
 
-// decodeClaims decodes the raw JWT into claims.
 func decodeClaims[T jwt.Claims](publicKey *rsa.PublicKey, raw string, claims T) error {
 	_, err := jwt.ParseWithClaims(raw, claims, evalClaims(publicKey), jwt.WithValidMethods([]string{jwt.SigningMethodRS256.Alg()}))
 
 	return err
 }
 
-// evalClaims evaluates if a token is valid.
 func evalClaims(publicKey *rsa.PublicKey) jwt.Keyfunc {
 	return func(t *jwt.Token) (any, error) {
 		if _, ok := t.Method.(*jwt.SigningMethodRSA); !ok {

pkg/envs/envs.go

@@ -6,12 +6,19 @@ import (
 	"strings"
 )
 
+// Edition is the ShellHub edition an instance runs as. It decides which features the
+// server exposes, so it is read at startup and never per request.
 type Edition string
 
 const (
-	Community  Edition = "community"
+	// Community is the open-source edition, and the edition an instance falls back to when
+	// SHELLHUB_EDITION is unset.
+	Community Edition = "community"
+	// Enterprise is the self-hosted paid edition, gated by a licence file.
 	Enterprise Edition = "enterprise"
-	Cloud      Edition = "cloud"
+	// Cloud is the hosted edition, which adds billing and the multi-tenant surface on top of
+	// Enterprise.
+	Cloud Edition = "cloud"
 )
 
 // Backend is an interface for any sort of underlying key/value store.
@@ -58,26 +65,39 @@ func CurrentEdition() Edition {
 	return edition
 }
 
+// IsCommunity reports whether this instance runs the community edition. It panics on an
+// unrecognized SHELLHUB_EDITION, as CurrentEdition does.
 func IsCommunity() bool {
 	return CurrentEdition() == Community
 }
 
+// IsEnterprise reports whether this instance runs the enterprise edition, which is not the
+// same question as "may it use a paid feature" — see IsEnterpriseOrCloud.
 func IsEnterprise() bool {
 	return CurrentEdition() == Enterprise
 }
 
+// IsCloud reports whether this instance runs the hosted edition. Billing and namespace
+// limits are the features that turn on here and nowhere else.
 func IsCloud() bool {
 	return CurrentEdition() == Cloud
 }
 
+// IsEnterpriseOrCloud reports whether the paid feature set is available. This is the check a
+// feature gate wants, so that a feature added for cloud stays available to enterprise.
 func IsEnterpriseOrCloud() bool {
 	return CurrentEdition() != Community
 }
 
+// IsDevelopment reports whether SHELLHUB_ENV is "development". It gates developer conveniences
+// only; never use it to relax a security decision, as the variable is attacker-controlled in
+// any deployment that passes the environment through.
 func IsDevelopment() bool {
 	return DefaultBackend.Get("SHELLHUB_ENV") == "development"
 }
 
+// ErrParseWithPrefix is joined with the backend's error when ParseWithPrefix fails, so a
+// caller can tell a configuration problem from anything else with errors.Is.
 var ErrParseWithPrefix = errors.New("failed to parse environment variables for the given prefix")
 
 // ParseWithPrefix parses the environment variables for the a given prefix.
@@ -99,6 +119,8 @@ func ParseWithPrefix[T any](prefix string) (*T, error) {
 	return envs, nil
 }
 
+// ErrParse is joined with the backend's error when Parse fails, so a caller can tell a
+// configuration problem from anything else with errors.Is.
 var ErrParse = errors.New("failed to parse environment variables")
 
 // Parse parses the environment variables.

pkg/wsconnadapter/wsconnadapter.go

@@ -13,16 +13,19 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-// an adapter for representing WebSocket connection as a net.Conn
-// some caveats apply: https://github.com/gorilla/websocket/issues/441
-
+// ErrUnexpectedMessageType is returned by Read when the peer sends a frame that is not
+// a binary one. The adapter carries a byte stream, so any other type is a protocol error
+// rather than something to skip.
 var ErrUnexpectedMessageType = errors.New("unexpected websocket message type")
 
 const (
 	defaultPongTimeout  = time.Second * 35
 	defaultPingInterval = time.Second * 30
 )
 
+// Adapter is a net.Conn backed by a websocket connection. The zero value is not usable;
+// build one with New. Read and Write are each safe for concurrent use, Close is
+// idempotent, and the keep-alive loop starts only once Ping is called.
 type Adapter struct {
 	UUID       string
 	conn       *websocket.Conn
@@ -37,20 +40,23 @@ type Adapter struct {
 	Logger     *log.Entry
 	CreatedAt  time.Time
 
-	// pingInterval and pongTimeout control the keep-alive loop. They default to
-	// the package defaults and are only overridden in tests.
 	pingInterval time.Duration
 	pongTimeout  time.Duration
 }
 
+// Option configures an Adapter during New.
 type Option func(*Adapter)
 
+// WithID sets the adapter's UUID, which the caller uses to correlate the connection with
+// its own bookkeeping. It is not read by the adapter itself.
 func WithID(id string) Option {
 	return func(a *Adapter) {
 		a.UUID = id
 	}
 }
 
+// WithDevice tags every log line the adapter emits with the tenant and device it belongs
+// to, so a connection can be followed through the logs of a busy server.
 func WithDevice(tenant string, device string) Option {
 	return func(a *Adapter) {
 		a.Logger = a.Logger.WithFields(log.Fields{
@@ -60,6 +66,9 @@ func WithDevice(tenant string, device string) Option {
 	}
 }
 
+// New wraps conn in an Adapter. The adapter takes ownership of conn: closing the adapter
+// closes it, and the caller must not use conn directly afterwards. Keep-alive is off until
+// Ping is called.
 func New(conn *websocket.Conn, options ...Option) *Adapter {
 	adapter := &Adapter{
 		conn: conn,
@@ -81,6 +90,14 @@ func New(conn *websocket.Conn, options ...Option) *Adapter {
 	return adapter
 }
 
+// Ping starts the keep-alive loop and returns the channel each pong is announced on. It is
+// idempotent: later calls return the same channel without starting a second loop. The
+// channel is not buffered and a pong is dropped rather than blocking the read loop, so a
+// caller that stops receiving slows nothing down.
+//
+// A failed ping write is terminal — a broken pipe or a closed socket — so the adapter closes
+// itself, which propagates teardown to whoever is reading. Missing pongs for pongTimeout has
+// the same effect.
 func (a *Adapter) Ping() chan bool {
 	a.pingOnce.Do(func() {
 		a.stopPingCh = make(chan struct{})
@@ -113,8 +130,6 @@ func (a *Adapter) Ping() chan bool {
 				select {
 				case <-ticker.C:
 					if err := a.conn.WriteControl(websocket.PingMessage, []byte{}, time.Now().Add(5*time.Second)); err != nil { //nolint:forbidigo // a deadline or an elapsed-time measurement needs the wall clock
-						// A failed ping write is terminal (broken pipe / closed socket):
-						// close the adapter so teardown propagates to the consumer, and stop.
 						a.Logger.
 							WithError(err).
 							WithField("lifetime", clock.Now().Sub(a.CreatedAt).String()).
@@ -136,8 +151,13 @@ func (a *Adapter) Ping() chan bool {
 	return a.pongCh
 }
 
+// Read fills b from the current websocket frame and is safe for concurrent use — it holds a
+// mutex because it advances the reader the next call resumes from.
+//
+// A frame boundary is not the end of the stream: the adapter's semantics are a byte stream
+// spread over many frames, so an io.EOF from the frame currently being read is reported as a
+// nil error and the next call opens the following frame.
 func (a *Adapter) Read(b []byte) (int, error) {
-	// Read() can be called concurrently, and we mutate some internal state here
 	a.readMutex.Lock()
 	defer a.readMutex.Unlock()
 
@@ -158,10 +178,7 @@ func (a *Adapter) Read(b []byte) (int, error) {
 	if err != nil {
 		a.reader = nil
 
-		// EOF for the current Websocket frame, more will probably come so..
 		if errors.Is(err, io.EOF) {
-			// .. we must hide this from the caller since our semantics are a
-			// stream of bytes across many frames
 			err = nil
 		}
 	}
@@ -173,6 +190,8 @@ func (a *Adapter) Read(b []byte) (int, error) {
 	return bytesRead, err
 }
 
+// Write sends b as a single binary frame and is safe for concurrent use. A short write is
+// reported as it happened: the count returned is what the frame took.
 func (a *Adapter) Write(b []byte) (int, error) {
 	a.writeMutex.Lock()
 	defer a.writeMutex.Unlock()
@@ -194,6 +213,8 @@ func (a *Adapter) Write(b []byte) (int, error) {
 	return bytesWritten, err
 }
 
+// Close stops the keep-alive loop and closes the underlying connection. It is idempotent —
+// every call after the first returns the error the first one produced.
 func (a *Adapter) Close() error {
 	a.closeOnce.Do(func() {
 		if a.stopPingCh != nil {
@@ -207,14 +228,19 @@ func (a *Adapter) Close() error {
 	return a.closeErr
 }
 
+// LocalAddr returns the local address of the underlying websocket connection.
 func (a *Adapter) LocalAddr() net.Addr {
 	return a.conn.LocalAddr()
 }
 
+// RemoteAddr returns the peer address of the underlying websocket connection. Behind a proxy
+// this is the proxy, not the device.
 func (a *Adapter) RemoteAddr() net.Addr {
 	return a.conn.RemoteAddr()
 }
 
+// SetDeadline applies t to both directions, and returns on the first failure — so a failure
+// on the read side leaves the write deadline unchanged.
 func (a *Adapter) SetDeadline(t time.Time) error {
 	if err := a.SetReadDeadline(t); err != nil {
 		a.Logger.WithError(err).Trace("failed to set the deadline")
@@ -225,10 +251,13 @@ func (a *Adapter) SetDeadline(t time.Time) error {
 	return a.SetWriteDeadline(t)
 }
 
+// SetReadDeadline applies t to the read side.
 func (a *Adapter) SetReadDeadline(t time.Time) error {
 	return a.conn.SetReadDeadline(t)
 }
 
+// SetWriteDeadline applies t to the write side. It takes the write mutex, so it waits for an
+// in-flight Write rather than racing it.
 func (a *Adapter) SetWriteDeadline(t time.Time) error {
 	a.writeMutex.Lock()
 	defer a.writeMutex.Unlock()