feat(version): GitHub OAuth author verification for version commit#78
Merged
feat(version): GitHub OAuth author verification for version commit#78
Conversation
- Add internal/version/auth.go: GitHub device flow, LoadIdentity/SaveIdentity - version init prompts for GitHub auth; stores verified username in .deepdiffdb/config - version init --skip-auth bypasses prompt for CI/scripted environments - version commit reads identity from config; github:<username> used as author automatically - --author flag still works when no config identity exists (backward compat) - DEEPDIFFDB_GITHUB_CLIENT_ID env var or build-time -ldflags injection for client ID - 5 new unit tests in tests/version/auth_test.go Closes #77
…eature - CHANGELOG.md: add [Unreleased] entry for issue #77 - ROADMAP.md: mark GitHub auth as in-progress, strikethrough completed versioning items - README.md: update version workflow — no --author needed when authenticated - website/docs/commands/version.md: add --skip-auth flag, author resolution order, GitHub OAuth setup guide, updated storage layout with config file - website/docs/features/git-versioning.md: add verified authorship section, update repo layout diagram and typical workflow
github-actions
bot
added
documentation
Contributor
📝 WalkthroughWalkthroughThis change adds GitHub OAuth device-flow to Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as deepdiffdb CLI
participant GitHub as GitHub API
participant FS as File System
User->>CLI: deepdiffdb version init
CLI->>User: Prompt: "Authenticate with GitHub?"
User->>CLI: Yes
CLI->>GitHub: POST /login/device/code (client_id)
GitHub-->>CLI: device_code, user_code, verification_uri
CLI->>User: Print verification_uri & user_code
Note over User,GitHub: User authorizes via verification_uri
loop Poll token endpoint
CLI->>GitHub: POST /login/oauth/access_token (device_code)
GitHub-->>CLI: authorization_pending / slow_down / access_token / error
end
CLI->>GitHub: GET /user (Bearer access_token)
GitHub-->>CLI: { login: username }
CLI->>FS: Save { github_user: username } to .deepdiffdb/config (rgba(0,128,0,0.5))
Note over CLI,FS: Later on commit
User->>CLI: deepdiffdb version commit --message "..."
CLI->>FS: Load .deepdiffdb/config
FS-->>CLI: { github_user: username }
CLI->>CLI: Set author = "github:username"
CLI->>FS: Write version commit with author
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/deepdiffdb/main.go (1)
1574-1581:⚠️ Potential issue | 🟠 MajorDon’t return before auth setup on an existing repo.
This makes the new identity flow unreachable once
.deepdiffdb/already exists. A cloned repo that shares.deepdiffdb/but keeps.deepdiffdb/configlocal, a repo first initialized with--skip-auth, or even a first run that failed after creating the repo has no supported way to add a verified identity later.🔁 Suggested fix
- if vcs.IsInitialized(*dir) { - fmt.Println("Version repository already initialised.") - return nil - } - if err := vcs.Init(*dir); err != nil { - return err - } - fmt.Printf("Initialised version repository in %s/%s\n", *dir, vcs.RepoDirName) + if vcs.IsInitialized(*dir) { + fmt.Println("Version repository already initialised.") + } else { + if err := vcs.Init(*dir); err != nil { + return err + } + fmt.Printf("Initialised version repository in %s/%s\n", *dir, vcs.RepoDirName) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/deepdiffdb/main.go` around lines 1574 - 1581, The current branch returns early when vcs.IsInitialized(*dir) is true, preventing the subsequent auth/identity setup from running for existing repos; change the control flow so that you do not return on an already-initialized repo—only call vcs.Init(*dir) when !vcs.IsInitialized(*dir), otherwise print the "already initialised" message and continue into the auth/identity setup logic; keep the existing error handling for vcs.Init and still print the success message using vcs.RepoDirName when Init runs.
🧹 Nitpick comments (2)
website/docs/commands/version.md (2)
402-402: Minor formatting inconsistency.Line 402 is missing a colon after "identity" while line 72 has it. For consistency:
✨ Suggested fix
- config ← verified identity {"github_user": "..."} (0o600) + config ← verified identity: {"github_user": "..."} (0o600)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/commands/version.md` at line 402, Update the text entry "config ← verified identity {"github_user": "..."} (0o600)" to include a colon after "identity" so it reads "config ← verified identity: {"github_user": "..."} (0o600)" to match the formatting used elsewhere (e.g., the line at 72).
117-117: Clarify the--authordefault value.The "Default" column showing
$USERcould be misleading because the actual author resolution is a multi-step priority chain (documented in lines 123-131), not a simple default. Consider changing the default to something like_(resolved, see below)_or_(see author resolution)_to direct users to the detailed explanation.📝 Suggested clarification
-| `--author` | `$USER` | Author name — **ignored** when GitHub identity is stored in `.deepdiffdb/config` | +| `--author` | _(see author resolution)_ | Author name — **ignored** when GitHub identity is stored in `.deepdiffdb/config` |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/commands/version.md` at line 117, Update the table row for the `--author` flag so the "Default" column no longer shows a literal `$USER`; replace it with a clarifying placeholder such as "_(resolved, see author resolution below)_", "_(see author resolution)_", or similar that points readers to the multi-step author resolution described later (lines describing author resolution), and ensure the `--author` row text still notes that the flag is ignored when GitHub identity is stored in `.deepdiffdb/config`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 16-17: Update the CHANGELOG entry that currently says "5 new unit
tests in `tests/version/auth_test.go`" to the correct count "6 new unit tests in
`tests/version/auth_test.go`" so the changelog matches the actual additions;
verify the line referencing `internal/version/auth.go` remains unchanged and
commit the corrected single-line entry.
In `@cmd/deepdiffdb/main.go`:
- Around line 1706-1725: The code currently falls back to os.Getenv("USER" )
when neither vcs.LoadIdentity(dir) returns a verified identity nor the --author
flag is provided; remove that implicit $USER fallback so the contract becomes
“verified identity or explicit --author”: in the block handling authorName
(after calling vcs.LoadIdentity and when authorName == ""), stop reading USER
from the environment and instead immediately return the existing error
fmt.Errorf("no author set: authenticate with 'version init' or pass --author
<name>") if *author == ""; ensure authorName is only set to *author when
provided and keep the "github:"+ prefix and warning behavior when a verified
identity exists.
In `@internal/version/auth.go`:
- Around line 69-70: The SaveIdentity write uses os.WriteFile(path,
append(data,'\n'), 0o600) which only applies mode on create—add an explicit
os.Chmod(path, 0o600) immediately after the WriteFile call (checking and
returning any error) to enforce owner-only perms on every overwrite; update the
TestSaveIdentityOverwrite test to assert file mode is 0600 after the second
SaveIdentity call.
In `@tests/version/auth_test.go`:
- Around line 123-132: After clearing the env var, explicitly assert the
fallback by setting the package-level GitHubClientID to a known value, call
vcs.ResolveClientID() and compare it to that known value; make sure to save the
original GitHubClientID before changing it and restore it after the assertion to
avoid test interference. Use the existing symbols envKey, vcs.ResolveClientID(),
and the GitHubClientID package variable to locate where to inject the
setup/assertion.
In `@website/docs/features/git-versioning.md`:
- Around line 63-66: Update the doc example to show that .deepdiffdb/config
stores the raw GitHub username under the key github_user (e.g., github_user:
"iamvirul") and clarify that the "github:" prefix is not stored on disk but is
added later by the version commit flow (deepdiffdb version init and deepdiffdb
version commit). Replace the current example line that implies "github:iamvirul"
is written to disk with the raw config value and a short note that the prefixed
author string is derived at commit time.
---
Outside diff comments:
In `@cmd/deepdiffdb/main.go`:
- Around line 1574-1581: The current branch returns early when
vcs.IsInitialized(*dir) is true, preventing the subsequent auth/identity setup
from running for existing repos; change the control flow so that you do not
return on an already-initialized repo—only call vcs.Init(*dir) when
!vcs.IsInitialized(*dir), otherwise print the "already initialised" message and
continue into the auth/identity setup logic; keep the existing error handling
for vcs.Init and still print the success message using vcs.RepoDirName when Init
runs.
---
Nitpick comments:
In `@website/docs/commands/version.md`:
- Line 402: Update the text entry "config ← verified
identity {"github_user": "..."} (0o600)" to include a colon after "identity" so
it reads "config ← verified identity: {"github_user":
"..."} (0o600)" to match the formatting used elsewhere (e.g., the line at 72).
- Line 117: Update the table row for the `--author` flag so the "Default" column
no longer shows a literal `$USER`; replace it with a clarifying placeholder such
as "_(resolved, see author resolution below)_", "_(see author resolution)_", or
similar that points readers to the multi-step author resolution described later
(lines describing author resolution), and ensure the `--author` row text still
notes that the flag is ignored when GitHub identity is stored in
`.deepdiffdb/config`.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c4bc07b-2e4c-4522-88a9-4426aa3f2bde
📒 Files selected for processing (9)
CHANGELOG.mdREADME.mdROADMAP.mdcmd/deepdiffdb/main.gointernal/version/auth.gotests/version/auth_test.gowebsite/docs/commands/version.mdwebsite/docs/deployment/cicd.mdwebsite/docs/features/git-versioning.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/version/auth.go (1)
68-71:⚠️ Potential issue | 🟠 MajorEnforce
0600on every write, not only file creation.
SaveIdentitycurrently relies onos.WriteFile(..., 0o600), which does not reset mode on overwrite. If permissions drift, Line 69 preserves insecure mode and breaks the owner-only guarantee.Suggested fix
path := filepath.Join(dir, RepoDirName, configFileName) if err := os.WriteFile(path, append(data, '\n'), 0o600); err != nil { // `#nosec` G306 return fmt.Errorf("writing identity config: %w", err) } +if err := os.Chmod(path, 0o600); err != nil { + return fmt.Errorf("setting identity config permissions: %w", err) +} return nil#!/bin/bash set -euo pipefail # Verify SaveIdentity currently writes but does/doesn't enforce chmod afterward. rg -n -C2 'func SaveIdentity|WriteFile|Chmod' internal/version/auth.go # Verify overwrite test coverage includes permission assertion after overwrite. rg -n -C3 'TestSaveIdentityOverwrite|0o600|Mode\(\)\.Perm\(\)' tests/version/auth_test.go🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/version/auth.go` around lines 68 - 71, SaveIdentity currently calls os.WriteFile(path, ..., 0o600) which does not change the mode on an existing file; ensure owner-only permissions are enforced on every write by calling os.Chmod(path, 0o600) immediately after the WriteFile succeeds. Update the SaveIdentity implementation (the block that constructs path using RepoDirName and configFileName and writes the identity file) to perform a chmod on path after successful write and return any chmod error wrapped similarly to the existing fmt.Errorf pattern.
🧹 Nitpick comments (2)
internal/version/auth.go (2)
54-57: Normalize loaded username before returning it.
LoadIdentityreturns rawcfg.GitHubUser; whitespace-only values are treated as authenticated downstream and becomegithub:. Trim before returning.Suggested fix
var cfg IdentityConfig if err := json.Unmarshal(data, &cfg); err != nil { return "", fmt.Errorf("parsing identity config: %w", err) } - return cfg.GitHubUser, nil + return strings.TrimSpace(cfg.GitHubUser), nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/version/auth.go` around lines 54 - 57, LoadIdentity currently returns the raw cfg.GitHubUser which can be whitespace-only; update LoadIdentity to normalize the username by trimming surrounding whitespace (e.g., strings.TrimSpace) on cfg.GitHubUser before returning and treat an empty result as unauthenticated (return empty string or error as existing semantics expect). Ensure you modify the return flow in LoadIdentity so the trimmed value is returned instead of the raw cfg.GitHubUser.
14-25: Wire build-timeGitHubClientIDinjection into release pipelines.This file supports ldflags injection, but release/build configs should consistently pass
-X github.com/iamvirul/deepdiff-db/internal/version.GitHubClientID=...; otherwise binaries default to auth-disabled unless env vars are set at runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/version/auth.go` around lines 14 - 25, Build pipelines aren't injecting the GitHubClientID via ldflags, leaving GitHubClientID empty at runtime; update your release/build configs to pass -X github.com/iamvirul/deepdiff-db/internal/version.GitHubClientID=<id> into go build (or go build -ldflags) for every build target and CI job that produces binaries so the injected value replaces the package-level GitHubClientID variable; ensure quoting/escaping is correct for your shell/CI, and keep the runtime environment variable behavior as the precedence mechanism if desired.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/version/auth.go`:
- Line 218: The error message string "device code expired — run `version init
--auth` again" is incorrect because the CLI uses an opt-out flag `--skip-auth`;
update the message in internal/version/auth.go to instruct users to run the
default auth flow (e.g., "run `version init`" or "run `version init` and do not
use --skip-auth") instead of referencing `--auth`, so replace the existing
literal with a corrected hint that mentions `version init` (and optionally
explicitly note to omit `--skip-auth`).
---
Duplicate comments:
In `@internal/version/auth.go`:
- Around line 68-71: SaveIdentity currently calls os.WriteFile(path, ..., 0o600)
which does not change the mode on an existing file; ensure owner-only
permissions are enforced on every write by calling os.Chmod(path, 0o600)
immediately after the WriteFile succeeds. Update the SaveIdentity implementation
(the block that constructs path using RepoDirName and configFileName and writes
the identity file) to perform a chmod on path after successful write and return
any chmod error wrapped similarly to the existing fmt.Errorf pattern.
---
Nitpick comments:
In `@internal/version/auth.go`:
- Around line 54-57: LoadIdentity currently returns the raw cfg.GitHubUser which
can be whitespace-only; update LoadIdentity to normalize the username by
trimming surrounding whitespace (e.g., strings.TrimSpace) on cfg.GitHubUser
before returning and treat an empty result as unauthenticated (return empty
string or error as existing semantics expect). Ensure you modify the return flow
in LoadIdentity so the trimmed value is returned instead of the raw
cfg.GitHubUser.
- Around line 14-25: Build pipelines aren't injecting the GitHubClientID via
ldflags, leaving GitHubClientID empty at runtime; update your release/build
configs to pass -X
github.com/iamvirul/deepdiff-db/internal/version.GitHubClientID=<id> into go
build (or go build -ldflags) for every build target and CI job that produces
binaries so the injected value replaces the package-level GitHubClientID
variable; ensure quoting/escaping is correct for your shell/CI, and keep the
runtime environment variable behavior as the precedence mechanism if desired.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
- CHANGELOG: correct test count from 5 → 6 unit tests - auth.go: add os.Chmod(0o600) after WriteFile so overwrites preserve permissions - main.go: remove $USER fallback in author resolution — explicit author or verified identity required; ambiguous fallback undermined the verification contract - main.go: fix runVersionInit early return — existing repo now continues to auth prompt, enabling users to add GitHub identity to a repo initialised with --skip-auth - auth_test.go: TestResolveClientID now explicitly asserts GitHubClientID fallback value instead of just asserting no panic; TestSaveIdentityOverwrite now verifies 0600 permissions are preserved after an overwrite - git-versioning.md: fix misleading comment — config stores plain username JSON, not "github:<username>" prefix (prefix is added at commit time) - blog: add closing line
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #77
Implements GitHub OAuth device flow authentication for the versioning system. Verified GitHub identity is stored in
.deepdiffdb/configand used automatically as the commit author — no--authorflag required.Changes
Code
internal/version/auth.go— full GitHub device flow:RunGitHubDeviceFlow,LoadIdentity,SaveIdentity,ResolveClientIDcmd/deepdiffdb/main.go—version initprompts for GitHub auth;version commitresolves author from config firstversion init --skip-authflag for CI/scripted environmentsDEEPDIFFDB_GITHUB_CLIENT_IDenv var or build-time-ldflagsinjectionGET /api.github.com/userTests
tests/version/auth_test.go— 5 new tests: round-trip save/load, missing file,0o600permissions, overwrite, corrupt JSON,ResolveClientIDenv var priorityDocs
CHANGELOG.md—[Unreleased]entryROADMAP.md— GitHub Auth marked doneREADME.md— version workflow updated; no--authorneeded when authenticatedwebsite/docs/commands/version.md—--skip-authflag, author resolution order, GitHub OAuth App setup guidewebsite/docs/features/git-versioning.md— verified authorship section, updated storage layoutwebsite/docs/deployment/cicd.md—version commitin CI example with--skip-authTest plan
go test ./tests/version/...— all 47+ tests passdeepdiffdb version init— prompts for GitHub auth whenDEEPDIFFDB_GITHUB_CLIENT_IDis setdeepdiffdb version init --skip-auth— skips prompt, no errordeepdiffdb version commitafter auth — usesgithub:<username>, no--authorneededdeepdiffdb version commit --author Xafter auth — warning printed, config identity winsdeepdiffdb version commitwithout auth and no--author— returns errorSetup required
Before end-to-end testing register a GitHub OAuth App with Device Flow enabled and set
DEEPDIFFDB_GITHUB_CLIENT_ID. See the GitHub OAuth setup guide in the docs.Summary by CodeRabbit
New Features
version initperforms interactive GitHub device-flow auth (can be skipped with--skip-auth)version commitauto-attributes commits asgithub:<username>when verified;--authoris ignored in that caseDocumentation
Tests