Refactor auth models to use concrete auth.Actor#1
Conversation
…lls to decouple auth modules
|
Need the big picture first? Review this PR in Change Stack to see what changed before going file by file. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a persistent auth.Actor model and migrates API key, email/password, and JWT flows to use the concrete type and accessor methods; exports CLI runners, adjusts module/CLI registration, adds CHANGELOG.md and SECURITY.md, updates CI to checkout local mdk, and updates CodeRabbit config. ChangesAuth Actor Model and Provider Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
emailpass/module.go (1)
84-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
auth.Actorto the module model list.The CLI path now migrates
auth.Actoron Line 31 inemailpass/cli.go, but this module still exposes onlyUserand the JWT tables. That splits schema creation by entrypoint and can leave the server path without the persisted actors table.Proposed fix
import ( "context" "encoding/json" "fmt" "os" "strings" "time" + "github.com/GoHyperrr/auth" "github.com/GoHyperrr/auth/jwt" "github.com/GoHyperrr/mdk" "github.com/google/uuid" "gorm.io/gorm" ) @@ func (m *Module) Models() []any { return []any{ + &auth.Actor{}, &User{}, &jwt.RefreshToken{}, &jwt.Blacklist{}, } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@emailpass/module.go` around lines 84 - 89, The Models() method currently returns &User, &jwt.RefreshToken, and &jwt.Blacklist but omits auth.Actor which causes split schema creation; update Module.Models() to include &auth.Actor in the returned slice (alongside &User, &jwt.RefreshToken, &jwt.Blacklist) so the actor table is migrated by this module regardless of CLI/Server entrypoint.jwt/jwt.go (1)
123-148:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReload the persisted actor during token validation.
ValidateTokennow synthesizes anauth.Actorfrom claims only, soNameandMetadataare always empty on token-authenticated requests. That flows straight intoemailpass.Me, and it also means tokens remain usable even if the backing actor row has been removed. After the blacklist check, load the actor from the database byclaims.ActorIDand return that record instead.Proposed fix
func (s *AuthStore) ValidateToken(ctx context.Context, tokenString string) (mdk.Actor, error) { token, err := jwt.ParseWithClaims(tokenString, &Claims{}, func(token *jwt.Token) (interface{}, error) { if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok { return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) } return s.signingKey, nil }) @@ if claims, ok := token.Claims.(*Claims); ok && token.Valid { // Check blacklist revoked, err := s.IsBlacklisted(ctx, claims.ID) if err != nil { return nil, fmt.Errorf("failed to check token revocation: %w", err) } if revoked { return nil, errors.New("token is revoked") } - return &auth.Actor{ - ID: claims.ActorID, - Type: claims.ActorType, - }, nil + var actor auth.Actor + if err := s.db.WithContext(ctx).First(&actor, "id = ?", claims.ActorID).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, ErrInvalidToken + } + return nil, fmt.Errorf("failed to load actor: %w", err) + } + return &actor, nil } return nil, ErrInvalidToken }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@jwt/jwt.go` around lines 123 - 148, ValidateToken currently returns a synthesized auth.Actor from Claims (leaving Name/Metadata empty) and doesn't ensure the actor still exists; after the blacklist check in ValidateToken, load the persisted actor record by claims.ActorID (e.g. call an existing store method such as s.GetActorByID(ctx, claims.ActorID) or s.GetActor(ctx, claims.ActorID)), return that actor instead of constructing a new &auth.Actor, and handle/propagate errors (including not-found) with proper wrapping; keep the existing revocation check using IsBlacklisted and ensure Claims and claims.ActorID are used to fetch the DB record so Name and Metadata are populated for token-authenticated requests.apikey/cli.go (1)
34-43:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle seed query errors and avoid count/create race.
Line 34 ignores DB errors, and the
Count→Createflow is racy under concurrent runs. UseFirstOrCreatewith explicit error handling for idempotent seeding.Suggested change
- var actorCount int64 - database.Model(&auth.Actor{}).Where("id = ?", "act_mcp_developer").Count(&actorCount) - if actorCount == 0 { - devActor := auth.Actor{ - ID: "act_mcp_developer", - Type: mdk.ActorAIAgent, - Name: "Developer Agent", - } - if err := database.Create(&devActor).Error; err != nil { - return fmt.Errorf("failed to create developer actor: %w", err) - } + devActor := auth.Actor{ + ID: "act_mcp_developer", + Type: mdk.ActorAIAgent, + Name: "Developer Agent", + } + if err := database.Where("id = ?", "act_mcp_developer").FirstOrCreate(&devActor).Error; err != nil { + return fmt.Errorf("failed to ensure developer actor: %w", err) + } + if devActor.ID == "act_mcp_developer" { fmt.Println("Created Developer Agent actor (act_mcp_developer).") - } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apikey/cli.go` around lines 34 - 43, The current seeding ignores errors from database.Model(...).Where(...).Count and is racy between Count and database.Create; replace the Count + conditional Create logic with a single idempotent FirstOrCreate call on the auth.Actor model (e.g., construct devActor with ID "act_mcp_developer", Type mdk.ActorAIAgent, Name "Developer Agent" and call database.Where("id = ?", devActor.ID).FirstOrCreate(&devActor)), and check/return any error from that operation so failures are handled instead of being ignored; remove the separate Count and database.Create usage to avoid the race.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@actor.go`:
- Line 6: actor.go imports the local-replaced module "github.com/GoHyperrr/mdk"
which fails because go.mod contains a replace to ../mdk that isn't in the
checkout; fix by updating go.mod to remove or change the replace for
github.com/GoHyperrr/mdk to the published semantic version (or ensure the ../mdk
source is included in CI/checkout), then run `go mod tidy` to regenerate go.sum
and ensure imports in actor.go resolve cleanly.
In `@CHANGELOG.md`:
- Around line 7-8: Markdown lint MD022 requires a blank line after a heading;
open CHANGELOG.md and insert a single empty line immediately after the "###
Added" heading (the line containing "### Added") so there is a blank line
between that heading and the following list item ("- Concrete auth.Actor
persistence model...").
In `@emailpass/model.go`:
- Around line 16-17: User.Actor is a value (non-pointer) association so when
Preload("Actor") finds no row it stays zero-value and Login returns &user.Actor
which lets GenerateToken mint JWTs with empty actor_id/type; change User.Actor
to a pointer (auth.Actor) and update the GORM tag to match (e.g., Actor
*auth.Actor `gorm:"foreignKey:ActorID"`), then in the Login handler
(emailpass/handlers.go) add a check that user.Actor != nil and user.Actor.ID ==
user.ActorID (or reject) before returning the user so GenerateToken never
receives an empty/mismatched actor.
In `@go.mod`:
- Line 19: The go.mod contains a local replace directive "replace
github.com/GoHyperrr/mdk => ../mdk" which forces a sibling path and breaks CI;
remove that replace line so the module resolves via the declared version, and if
local development linking is needed use a go.work file instead; locate and
delete the replace directive referencing github.com/GoHyperrr/mdk (and update
any developer docs to suggest go work for local overrides).
---
Outside diff comments:
In `@apikey/cli.go`:
- Around line 34-43: The current seeding ignores errors from
database.Model(...).Where(...).Count and is racy between Count and
database.Create; replace the Count + conditional Create logic with a single
idempotent FirstOrCreate call on the auth.Actor model (e.g., construct devActor
with ID "act_mcp_developer", Type mdk.ActorAIAgent, Name "Developer Agent" and
call database.Where("id = ?", devActor.ID).FirstOrCreate(&devActor)), and
check/return any error from that operation so failures are handled instead of
being ignored; remove the separate Count and database.Create usage to avoid the
race.
In `@emailpass/module.go`:
- Around line 84-89: The Models() method currently returns &User,
&jwt.RefreshToken, and &jwt.Blacklist but omits auth.Actor which causes split
schema creation; update Module.Models() to include &auth.Actor in the returned
slice (alongside &User, &jwt.RefreshToken, &jwt.Blacklist) so the actor table is
migrated by this module regardless of CLI/Server entrypoint.
In `@jwt/jwt.go`:
- Around line 123-148: ValidateToken currently returns a synthesized auth.Actor
from Claims (leaving Name/Metadata empty) and doesn't ensure the actor still
exists; after the blacklist check in ValidateToken, load the persisted actor
record by claims.ActorID (e.g. call an existing store method such as
s.GetActorByID(ctx, claims.ActorID) or s.GetActor(ctx, claims.ActorID)), return
that actor instead of constructing a new &auth.Actor, and handle/propagate
errors (including not-found) with proper wrapping; keep the existing revocation
check using IsBlacklisted and ensure Claims and claims.ActorID are used to fetch
the DB record so Name and Metadata are populated for token-authenticated
requests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 428edb04-4e2a-4eed-b569-3af03f0e126c
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
CHANGELOG.mdSECURITY.mdactor.goapikey/cli.goapikey/graphql.goapikey/handlers.goapikey/model.goapikey/module.goemailpass/cli.goemailpass/graphql.goemailpass/handlers.goemailpass/model.goemailpass/module.gogo.modjwt/jwt.go
💤 Files with no reviewable changes (1)
- apikey/module.go
| import ( | ||
| "time" | ||
|
|
||
| "github.com/GoHyperrr/mdk" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== go.mod =="
sed -n '1,200p' go.mod
echo
echo "== mdk dependency / replace directives =="
rg -n -C2 'github\.com/GoHyperrr/mdk|^\s*replace\s+.*mdk' go.mod go.sumRepository: GoHyperrr/auth
Length of output: 802
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== filesystem check: ../mdk =="
if [ -d ../mdk ]; then
ls -la ../mdk | head -n 50
else
echo "MISSING: ../mdk directory does not exist"
fi
echo
echo "== mdk references in go.sum =="
rg -n 'github\.com/GoHyperrr/mdk' go.sum || echo "No go.sum entries found for github.com/GoHyperrr/mdk"Repository: GoHyperrr/auth
Length of output: 215
Fix broken local module replacement for github.com/GoHyperrr/mdk (currently replace ... => ../mdk).
go.mod replaces github.com/GoHyperrr/mdk with ../mdk, but ../mdk is not present in this checkout and there are no go.sum entries for the module, so builds that import it (via actor.go) cannot resolve dependencies. Remove/adjust the replace to use the published module version (or add the mdk source to the repo/CI checkout) and run go mod tidy.
🧰 Tools
🪛 GitHub Actions: CI / 0_build.txt
[error] 6-6: go vet failed: replacement directory ../mdk does not exist for module github.com/GoHyperrr/mdk@v0.0.0-20260605044506-3d2ab0d97ca9.
🪛 GitHub Actions: CI / build
[error] 6-6: go vet ./... failed: actor.go:6:2: github.com/GoHyperrr/mdk@v0.0.0-20260605044506-3d2ab0d97ca9: replacement directory ../mdk does not exist
🪛 GitHub Check: build
[failure] 6-6:
github.com/GoHyperrr/mdk@v0.0.0-20260605044506-3d2ab0d97ca9: replacement directory ../mdk does not exist
🪛 golangci-lint (2.12.2)
[error] 6-6: could not import github.com/GoHyperrr/mdk (actor.go:6:2: github.com/GoHyperrr/mdk@v0.0.0-20260605044506-3d2ab0d97ca9: replacement directory ../mdk does not exist)
(typecheck)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@actor.go` at line 6, actor.go imports the local-replaced module
"github.com/GoHyperrr/mdk" which fails because go.mod contains a replace to
../mdk that isn't in the checkout; fix by updating go.mod to remove or change
the replace for github.com/GoHyperrr/mdk to the published semantic version (or
ensure the ../mdk source is included in CI/checkout), then run `go mod tidy` to
regenerate go.sum and ensure imports in actor.go resolve cleanly.
Sources: Linters/SAST tools, Pipeline failures
| ### Added | ||
| - Concrete auth.Actor persistence model implementing mdk.Actor interface. |
There was a problem hiding this comment.
Add a blank line after the “Added” heading to satisfy markdownlint MD022.
Insert one empty line between Line 7 and Line 8 to resolve the documented lint warning.
Suggested patch
### Added
+
- Concrete auth.Actor persistence model implementing mdk.Actor interface.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Added | |
| - Concrete auth.Actor persistence model implementing mdk.Actor interface. | |
| ### Added | |
| - Concrete auth.Actor persistence model implementing mdk.Actor interface. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` around lines 7 - 8, Markdown lint MD022 requires a blank line
after a heading; open CHANGELOG.md and insert a single empty line immediately
after the "### Added" heading (the line containing "### Added") so there is a
blank line between that heading and the following list item ("- Concrete
auth.Actor persistence model...").
Source: Linters/SAST tools
| ActorID string `gorm:"not null" json:"actor_id"` | ||
| Actor mdk.Actor `gorm:"foreignKey:ActorID" json:"actor"` | ||
| Actor auth.Actor `gorm:"foreignKey:ActorID" json:"actor"` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Files containing emailpass/model.go and emailpass/handlers.go =="
ls -la emailpass || true
git ls-files | rg -n '^emailpass/(model\.go|handlers\.go)$' || true
echo
echo "== emailpass/model.go =="
sed -n '1,120p' emailpass/model.go
echo
echo "== emailpass/handlers.go (or similarly named handlers) =="
if [ -f emailpass/handlers.go ]; then
sed -n '1,240p' emailpass/handlers.go
else
git ls-files | rg '^emailpass/.*handlers\.go$' || true
fi
echo
echo "== Search for Preload(\"Actor\") and references to user.Actor / ActorID =="
rg -n --hidden --no-ignore-vcs 'Preload\("Actor"\)|Preload\(\"Actor\"\)|\.Actor\b|GenerateToken|Login\(' emailpass || true
echo
echo "== Search for GenerateToken implementation =="
rg -n --hidden --no-ignore-vcs 'func .*GenerateToken|GenerateToken\(' . || trueRepository: GoHyperrr/auth
Length of output: 4700
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== emailpass/graphql.go =="
sed -n '1,220p' emailpass/graphql.go
echo
echo "== jwt/jwt.go =="
if [ -f jwt/jwt.go ]; then
sed -n '1,220p' jwt/jwt.go
else
git ls-files | rg -n '^jwt/.*\.go$' || true
fi
echo
echo "== emailpass/module.go =="
sed -n '1,240p' emailpass/module.go
echo
echo "== Search: returns &user.Actor / user.Actor usage =="
rg -n --hidden --no-ignore-vcs '&user\.Actor|\buser\.Actor\b' emailpass || true
echo
echo "== Search: actor_id constraints / foreign keys =="
rg -n --hidden --no-ignore-vcs 'ActorID|foreignKey|constraint|OnDelete|references' emailpass || trueRepository: GoHyperrr/auth
Length of output: 9363
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find auth.Actor type =="
git ls-files | rg -n '^.*auth/.*\.go$' || true
rg -n --hidden --no-ignore-vcs 'type Actor struct|func \(.*\) GetID\(\)|func \(.*\) GetType\(\)' . || true
echo
echo "== Read auth.Actor definition =="
# Try likely filenames
for f in $(git ls-files | rg '^auth/.*\.go$' || true); do
rg -n --hidden --no-ignore-vcs 'type Actor struct' "$f" >/dev/null 2>&1 && echo "$f"
done | head -n 20
# Print the first match file for Actor struct
actor_file="$(git ls-files | rg '^auth/.*\.go$' | while read -r f; do rg -l --hidden --no-ignore-vcs 'type Actor struct' "$f" >/dev/null && echo "$f"; done | head -n 1)"
if [ -n "${actor_file:-}" ] && [ -f "$actor_file" ]; then
echo
echo "== $actor_file (snippet) =="
nl -ba "$actor_file" | sed -n '1,220p'
else
echo "Could not locate auth/Actor struct file under auth/ directory."
fi
echo
echo "== Find where validate_actor workflow step is used =="
rg -n --hidden --no-ignore-vcs '"auth\.validate_actor"|validate_actor' . || true
echo
echo "== Search for token validation pipeline in runtime =="
rg -n --hidden --no-ignore-vcs 'ValidateToken|TokenValidator|Authenticate|auth\.emailpass' . || trueRepository: GoHyperrr/auth
Length of output: 256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== actor.go =="
nl -ba actor.go | sed -n '1,120p'
echo
echo "== Search for Login / GenerateToken usage in auth flow =="
rg -n --hidden --no-ignore-vcs 'LoginResolver|Store\(\)\.GenerateToken|GenerateToken\(|ValidateToken\(|TokenValidator|Authenticate|auth\.emailpass|emailpass\.Login|emailpass\.LoginResolver' . || true
echo
echo "== Search for mdk runtime authentication / middleware hook =="
rg -n --hidden --no-ignore-vcs 'Authenticate|token.*validate|ValidateToken|TokenValidator' . || trueRepository: GoHyperrr/auth
Length of output: 134
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== actor.go =="
cat -n actor.go | sed -n '1,140p'
echo
echo "== Search: Login/GenerateToken call sites =="
rg -n --hidden --no-ignore-vcs 'emailpass/|LoginResolver|emailpass\.Login|GenerateToken\(|GenerateToken\(' . || true
echo
echo "== Search: token validation / auth middleware =="
rg -n --hidden --no-ignore-vcs 'ValidateToken|TokenValidator|Authenticate|auth\.emailpass' . || true
echo
echo "== Search: where ValidateToken is called =="
rg -n --hidden --no-ignore-vcs '\.ValidateToken\(' . || trueRepository: GoHyperrr/auth
Length of output: 2665
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for validate_actor usage =="
rg -n --hidden --no-ignore-vcs 'validate_actor|ValidateActorStep|auth\.validate_actor' . || true
echo
echo "== Search for checks on empty actor_id / ActorID =="
rg -n --hidden --no-ignore-vcs 'ActorID.*(==|!=|empty|""|not null)|actor_id.*(==|!=|empty|"" )|claims\.ActorID' . || true
echo
echo "== Search for GetID() validation =="
rg -n --hidden --no-ignore-vcs '\.GetID\(\)' . || true
echo
echo "== Search for JWT claim parsing usage =="
rg -n --hidden --no-ignore-vcs 'ActorType|ActorID|RegisteredClaims|claims\.' jwt . emailpass . || trueRepository: GoHyperrr/auth
Length of output: 5205
Fix emailpass.User.Actor association to prevent issuing JWTs with empty actor_id/type
User.Actor is a non-pointer auth.Actor (value association). If Preload("Actor") can’t find the related row, GORM leaves user.Actor zero-valued; emailpass/handlers.go Login still returns &user.Actor, and GenerateToken then uses empty actor_id/actor_type when minting the JWT.
Suggested direction
type User struct {
ID string `gorm:"primaryKey" json:"id"`
Email string `gorm:"uniqueIndex;not null" json:"email"`
PasswordHash string `gorm:"not null" json:"-"`
ActorID string `gorm:"not null" json:"actor_id"`
- Actor auth.Actor `gorm:"foreignKey:ActorID" json:"actor"`
+ Actor *auth.Actor `gorm:"foreignKey:ActorID" json:"actor"`
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
DeletedAt gorm.DeletedAt `gorm:"index" json:"-"`
}Also reject login when the preloaded actor is nil (or when user.Actor.ID != user.ActorID) before returning it.
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
type User struct {
ID string `gorm:"primaryKey" json:"id"`
Email string `gorm:"uniqueIndex;not null" json:"email"`
PasswordHash string `gorm:"not null" json:"-"`
ActorID string `gorm:"not null" json:"actor_id"`
Actor *auth.Actor `gorm:"foreignKey:ActorID" json:"actor"`
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
DeletedAt gorm.DeletedAt `gorm:"index" json:"-"`
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@emailpass/model.go` around lines 16 - 17, User.Actor is a value (non-pointer)
association so when Preload("Actor") finds no row it stays zero-value and Login
returns &user.Actor which lets GenerateToken mint JWTs with empty actor_id/type;
change User.Actor to a pointer (auth.Actor) and update the GORM tag to match
(e.g., Actor *auth.Actor `gorm:"foreignKey:ActorID"`), then in the Login handler
(emailpass/handlers.go) add a check that user.Actor != nil and user.Actor.ID ==
user.ActorID (or reject) before returning the user so GenerateToken never
receives an empty/mismatched actor.
| golang.org/x/text v0.37.0 // indirect | ||
| ) | ||
|
|
||
| replace github.com/GoHyperrr/mdk => ../mdk |
There was a problem hiding this comment.
Remove committed local replace for mdk to avoid non-reproducible builds.
Line 19 pins github.com/GoHyperrr/mdk to a local ../mdk path. This commonly breaks CI and contributor builds when that sibling path is missing. Keep module resolution on the required version in go.mod and use go work for local linking instead.
Suggested change
-replace github.com/GoHyperrr/mdk => ../mdk📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| replace github.com/GoHyperrr/mdk => ../mdk |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` at line 19, The go.mod contains a local replace directive "replace
github.com/GoHyperrr/mdk => ../mdk" which forces a sibling path and breaks CI;
remove that replace line so the module resolves via the declared version, and if
local development linking is needed use a go.work file instead; locate and
delete the replace directive referencing github.com/GoHyperrr/mdk (and update
any developer docs to suggest go work for local overrides).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@SECURITY.md`:
- Line 5: Replace the ambiguous sentence that reads "For pre-1.0 releases we
support the latest release or the latest patch series (e.g., 0.1.x), and for
1.0+ we support the latest major release branch." with a clearer policy
statement: state that "For pre-1.0 releases we support all patch versions in the
latest minor series (e.g., all 0.1.x patches)." and that "For 1.0+ we support
all minor and patch versions within the latest major release (e.g., if 2.x is
current, we support all 2.y.z)." Ensure the revised wording replaces the
original ambiguous sentence in SECURITY.md.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b905abac-540c-4458-ad9b-122911ab257d
📒 Files selected for processing (1)
SECURITY.md
|
|
||
| ## Supported Versions | ||
|
|
||
| For pre-1.0 releases we support the latest release or the latest patch series (e.g., 0.1.x), and for 1.0+ we support the latest major release branch. |
There was a problem hiding this comment.
Clarify the supported versions policy.
The phrase "the latest release or the latest patch series" is ambiguous—readers may not understand whether you support only the newest patch (e.g., 0.1.3 but not 0.1.2) or all patches in the latest series (all 0.1.x). Similarly, "latest major release branch" for 1.0+ could be misread as supporting only one major version even after newer majors are released.
Consider rephrasing for clarity, for example:
- "For pre-1.0 releases, we support all patch versions in the latest minor series (e.g., all 0.1.x patches). For 1.0+, we support all minor and patch versions in the latest major release."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SECURITY.md` at line 5, Replace the ambiguous sentence that reads "For
pre-1.0 releases we support the latest release or the latest patch series (e.g.,
0.1.x), and for 1.0+ we support the latest major release branch." with a clearer
policy statement: state that "For pre-1.0 releases we support all patch versions
in the latest minor series (e.g., all 0.1.x patches)." and that "For 1.0+ we
support all minor and patch versions within the latest major release (e.g., if
2.x is current, we support all 2.y.z)." Ensure the revised wording replaces the
original ambiguous sentence in SECURITY.md.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 21: The checkout for the external repository GoHyperrr/mdk is using a
dynamic ref expression (ref: ${{ github.head_ref || github.ref_name }}) which
breaks for pull_request events because the source branch may not exist in that
repo; change the checkout step that targets repository: GoHyperrr/mdk to use a
stable ref (for example set ref to "main" or a pinned commit SHA) so CI
deterministically checks out a known branch/commit instead of the PR source
branch. Ensure you update the checkout step where repository: GoHyperrr/mdk and
ref is set (replace the expression with the chosen stable ref).
- Around line 16-18: The CI workflow is using mutable actions/checkout@v4 in two
steps (including the "Checkout mdk" step) and is not disabling credential
persistence or safely setting the ref; update both checkout steps to pin
actions/checkout to the corresponding full commit SHAs instead of `@v4`, add with:
persist-credentials: false to prevent the token from being written to git
config, and change the "Checkout mdk" step to avoid coupling to the auth repo
branch by using a stable ref fallback (e.g., a specific commit/branch or a safe
fallback instead of github.head_ref || github.ref_name) so the checkout won't
fail when the branch doesn't exist in GoHyperrr/mdk.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ee427c9a-bcd4-41f9-9caa-d777fd497436
📒 Files selected for processing (1)
.github/workflows/ci.yml
| uses: actions/checkout@v4 | ||
| - name: Checkout mdk | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ci.yml (lines 1-120) =="
sed -n '1,120p' .github/workflows/ci.yml | nl -ba
echo "== ci.yml (lines around 10-40) =="
sed -n '10,40p' .github/workflows/ci.yml | nl -ba
echo "== go.mod =="
cat go.mod | sed -n '1,200p' | nl -ba
echo "== Search for persist-credentials and checkout usage =="
rg -n "uses:\s*actions/checkout@|persist-credentials|GoHyperrr/mdk|ref:\s*\$\{\{|\bhead_ref\b|\bref_name\b" .github/workflows/ci.yml go.mod || true
echo "== Any other workflows with checkout? =="
rg -n "uses:\s*actions/checkout@" .github/workflows || trueRepository: GoHyperrr/auth
Length of output: 146
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflows tree =="
find .github/workflows -maxdepth 1 -type f -print || true
echo "== ci.yml =="
# show with line numbers using cat -n
cat -n .github/workflows/ci.yml | sed -n '1,140p'
echo "== go.mod (first 200 lines) =="
cat -n go.mod | sed -n '1,200p'
echo "== grep checkout + persist-credentials + GoHyperrr/mdk/ref derivation =="
rg -n "uses:\s*actions/checkout@|persist-credentials|GoHyperrr/mdk|ref:\s*\$\{\{|head_ref|ref_name" .github/workflows/ci.yml go.mod .github/workflows || true
echo "== exact checkout blocks (context) =="
rg -n "uses:\s*actions/checkout@" -C 3 .github/workflows/ci.yml .github/workflows || trueRepository: GoHyperrr/auth
Length of output: 3573
🌐 Web query:
actions/checkout persist-credentials default value
💡 Result:
The default value of the actions/checkout input persist-credentials is true.[1][2] This option controls whether the authentication token used to fetch the repository is persisted in the local git configuration so subsequent authenticated git commands work; it can be disabled with persist-credentials: false.[1][2]
Citations:
Pin actions/checkout to commit SHAs and disable persisted credentials (CI security).
- In
.github/workflows/ci.ymllines 16 and 18, both steps use mutableactions/checkout@v4; pin to the full commit SHA. - Neither checkout sets
with: persist-credentials: false;actions/checkoutdefaults this totrue, persisting the token in git config. - The
GoHyperrr/mdkcheckout usesref: ${{ github.head_ref || github.ref_name }}, coupling the ref to the auth repo’s branch name and risking failures when that branch doesn’t exist inGoHyperrr/mdk.
Proposed fix
- - name: Checkout auth
- uses: actions/checkout@v4
+ - name: Checkout auth
+ uses: actions/checkout@<FULL_LENGTH_COMMIT_SHA_FOR_V4>
...
- - name: Checkout mdk
- uses: actions/checkout@v4
+ - name: Checkout mdk
+ uses: actions/checkout@<FULL_LENGTH_COMMIT_SHA_FOR_V4>🧰 Tools
🪛 zizmor (1.25.2)
[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 16 - 18, The CI workflow is using
mutable actions/checkout@v4 in two steps (including the "Checkout mdk" step) and
is not disabling credential persistence or safely setting the ref; update both
checkout steps to pin actions/checkout to the corresponding full commit SHAs
instead of `@v4`, add with: persist-credentials: false to prevent the token from
being written to git config, and change the "Checkout mdk" step to avoid
coupling to the auth repo branch by using a stable ref fallback (e.g., a
specific commit/branch or a safe fallback instead of github.head_ref ||
github.ref_name) so the checkout won't fail when the branch doesn't exist in
GoHyperrr/mdk.
Source: Linters/SAST tools
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: GoHyperrr/mdk | ||
| ref: ${{ github.head_ref || github.ref_name }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la .github/workflows || true
sed -n '1,120p' .github/workflows/ci.yml | nl -ba | sed -n '1,120p'
echo "----"
# show around line 21 specifically
nl -ba .github/workflows/ci.yml | sed -n '10,40p'
echo "----"
# find all occurrences of github.head_ref / ref_name / checkout repo usage
rg -n "github\.head_ref|\bref_name\b|checkout:" .github/workflows/ci.yml
echo "----"
# print checkout block(s) for context
rg -n "uses:\s*actions/checkout|GoHyperrr/mdk|repository:" .github/workflows/ci.yml -nRepository: GoHyperrr/auth
Length of output: 283
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- workflows ----"
ls -la .github/workflows || true
echo "---- ci.yml (numbered via cat -n) ----"
cat -n .github/workflows/ci.yml | sed -n '1,120p'
echo "---- lines 15-35 ----"
sed -n '15,35p' .github/workflows/ci.yml
echo "---- checkout-related lines ----"
rg -n "actions/checkout|GoHyperrr/mdk|repository:|ref:\s*\$\{\{\s*github\.(head_ref|ref_name)\s*\|\|\s*github\.ref_name\s*\}\}" .github/workflows/ci.yml || true
echo "---- exact line containing github.head_ref/ref_name ----"
rg -n "github\.head_ref|\bgithub\.ref_name\b" .github/workflows/ci.yml || trueRepository: GoHyperrr/auth
Length of output: 2258
Make GoHyperrr/mdk checkout use a stable ref (PRs won’t have matching branch names)
In .github/workflows/ci.yml line 21, ref: ${{ github.head_ref || github.ref_name }} checks out GoHyperrr/mdk using the PR source branch name for pull_request events; that branch often won’t exist in GoHyperrr/mdk, causing the checkout to fail. Use a stable ref (e.g., main or a pinned commit/SHA) for deterministic CI.
Proposed fix
- ref: ${{ github.head_ref || github.ref_name }}
+ ref: main📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ref: ${{ github.head_ref || github.ref_name }} | |
| ref: main |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 21, The checkout for the external
repository GoHyperrr/mdk is using a dynamic ref expression (ref: ${{
github.head_ref || github.ref_name }}) which breaks for pull_request events
because the source branch may not exist in that repo; change the checkout step
that targets repository: GoHyperrr/mdk to use a stable ref (for example set ref
to "main" or a pinned commit SHA) so CI deterministically checks out a known
branch/commit instead of the PR source branch. Ensure you update the checkout
step where repository: GoHyperrr/mdk and ref is set (replace the expression with
the chosen stable ref).
1 participant
Refactors auth models to use concrete auth.Actor implementing mdk.Actor interface
Summary by CodeRabbit
New Features
Documentation
Chores