fix(time): don't inject default project on issue-scoped time list/log (#123)#124
Merged
Merged
Conversation
`time list --issue` and `time log --issue` applied the configured default project unconditionally, sending an unrelated project_id alongside the issue. Redmine authorizes /time_entries against that project_id and returns 403 when its time-tracking module is disabled (the admin bypass runs after the module check), or rejects the create on an issue/project mismatch. The equivalent raw API call, which sends only issue_id, worked, which is why the failure looked like a CLI-only permission bug. Gate the default-project fallback on issue == 0 so issue-scoped requests carry no injected project_id; an explicit --project is still resolved and honored. Project-scoped commands without an issue (including time summary) keep the default-project fallback, which is intended behavior. Adds unit regression tests for list, log, and summary, plus an e2e test covering a default project that differs from the issue's project. Closes #123
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
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
redmine time list --issue <id>andredmine time log --issue <id>returnedPermission denied(HTTP 403) while the equivalent raw callredmine api /time_entries.json -f issue_id=<id> ...worked with the same API key. Closes #123.Root cause
Both commands applied the configured default project unconditionally before building the request, injecting an unrelated
project_idthat the raw API call never sends:api /time_entries.json -f issue_id=42 -f limit=100(works)GET …?issue_id=42&limit=100time list --issue 42(default project set)GET …?issue_id=42&limit=100&offset=0&project_id=7time log --issue 42(default project set)POSTbody{"issue_id":42,"project_id":"7",…}Reproduced against a logging mock server, and the 403 was confirmed against Redmine's own source:
TimelogController#index->find_optional_project): authorization keys onproject_id, notissue_id.User#allowed_to?(action, project)runsreturn false unless context.allows_to?(action)(module-enabled check) beforereturn true if admin?, so aproject_idwhose project has time tracking disabled returns 403 even for an admin. With onlyissue_id, the:globalbranch grants admins immediately.TimelogController#create):TimeEntry#safe_attributes=only corrects the project to the issue's project whenproject_idis blank. A non-blank injectedproject_idkeeps the wrong project, thencreatedoesrender_403on thelog_timecheck (or fails validation with an issue/project mismatch).The reported "activity inactive at project level" detail is a red herring: that would surface as a 422, and
time listnever touches activities.Fix
Gate the default-project fallback on
issue == 0. When an issue scopes the request, the issue determines the project, so noproject_idis injected; an explicit--projectis still resolved and honored. Commands without an issue scope keep the default-project fallback.Scope check
A multi-agent audit of every default-project callsite plus the ops/MCP layers confirmed
time listandtime logare the only members of this bug class.time summarywas considered and deliberately left unchanged: it has no issue scope, so honoring the configured default project is intended (matching the no-issue branch of list/log); changing it would regress project-scoped summaries. The ops/MCP layers never inject a default project, so they are unaffected.Tests
internal/cmd/time/time_test.go):TestTimeList_IssueScopeIgnoresDefaultProjectandTestTimeLog_IssueScopeIgnoresDefaultProjectassert noproject_idleaks when scoping by issue with a default project configured.TestTimeSummary_HonorsDefaultProjectpins the deliberate decision that summary still honors the default project.e2e/time_entries_test.go):TestTimeEntries_DefaultProjectIgnoredForIssueScopeconfigures a default project that differs from the issue's project and verifiestime logattaches to the issue's project andtime list --issuereturns the entry.Verification
gofmt,go vet ./...(and-tags e2e),golangci-lint run ./...(0 issues)e2etag.