Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hide the "Details" link of commit status when the user cannot access actions #30156

Merged
merged 5 commits into from
Jul 28, 2024
Merged

Hide the "Details" link of commit status when the user cannot access actions #30156

merged 5 commits into from
Jul 28, 2024

Conversation

Zettat123
Copy link
Contributor

Fix #26685

If a commit status comes from Gitea Actions and the user cannot access the repo's actions unit (the user does not have the permission or the actions unit is disabled), a 404 page will occur after clicking the "Details" link. We should hide the "Details" link in this case.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 28, 2024
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 28, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Mar 28, 2024
@Zettat123 Zettat123 added type/bug topic/gitea-actions related to the actions of Gitea labels Mar 28, 2024
@silverwind silverwind added the backport/v1.22 This PR should be backported to Gitea 1.22 label Mar 28, 2024
lunny
lunny reviewed May 7, 2024
View reviewed changes
models/git/commit_status.go
}

prefix := fmt.Sprintf("%s/actions", status.Repo.Link())
if strings.HasPrefix(status.TargetURL, prefix) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is TargetURL a relative path or a URL?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends on the source of the commit status.

  • If the commit status comes from Gitea Actions, the TargetURL will be a relative path like /{owner}/{repo}/actions/runs/1/jobs/2.
  • If the commit status comes from an external service like Drone, the TargetURL will be a URL.

@yp05327
Copy link
Contributor

yp05327 commented May 16, 2024

Can this be hidden in SQL query?

@Zettat123
Copy link
Contributor Author

Can this be hidden in SQL query?

🤔 Are you saying that we shouldn't select TargetURL in the SQL query if the user doesn't have a read permission for actions?
It's feasible. The problem is that there are multiple functions related to query submission status. I'm worried that modifying them will bring too many changes to the callers of these functions. (In fact, I've made changes to a series of functions that need to query the commit status. So now I'm not sure which is the better solution)

@wxiaoguang
Copy link
Contributor

Since it is only a UI bug (404), which doesn't block daily usage, I think it is very trivial and doesn't need to backport. Too many lines are touched and we should avoid unnecessary regressions.

@wxiaoguang wxiaoguang removed the backport/v1.22 This PR should be backported to Gitea 1.22 label May 17, 2024
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 25, 2024
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 27, 2024
@wolfogre wolfogre added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 28, 2024
@wolfogre wolfogre merged commit 7dec8de into go-gitea:main Jul 28, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Jul 28, 2024
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 28, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 29, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
5481812 
* giteaofficial/main:
  Make GetRepositoryByName more safer (go-gitea#31712)
  [skip ci] Updated licenses and gitignores
  Run `go-install` in `deps-tools` in parallel (go-gitea#31711)
  Hide the "Details" link of commit status when the user cannot access actions (go-gitea#30156)
  Enable `no-jquery/no-parse-html-literal` and fix violation (go-gitea#31684)
  [skip ci] Updated translations via Crowdin
  OIDC: case-insensitive comparison for auth scheme `Basic` (go-gitea#31706)
  Support `pull_request_target` event for commit status (go-gitea#31703)
  Add types to fetch,toast,bootstrap,svg (go-gitea#31627)
  Run `detectWebAuthnSupport` only if necessary (go-gitea#31691)
  add `username` to OIDC introspection response (go-gitea#31688)
  Add return type to GetRawFileOrLFS and GetRawFile (go-gitea#31680)
  Support delete user email in admin panel (go-gitea#31690)
  Use GetDisplayName() instead of DisplayName() to generate rss feeds (go-gitea#31687)
@yp05327 yp05327 mentioned this pull request Jul 29, 2024
Merged
badhezi
badhezi reviewed Jul 29, 2024
View reviewed changes
routers/web/repo/repo.go
Copy link

@badhezi badhezi Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Zettat123
This invocation throws nil pointer dereference on my local setup.

2024/07/29 16:09:18 ...rs/common/errpage.go:26:RenderPanicErrorPage() [E] PANIC: runtime error: invalid memory address or nil pointer dereference
/usr/local/go/src/runtime/panic.go:770 (0x104ea8c43)
        gopanic: fn()
/Users/hezi/Documents/repos/hub/modules/web/routing/logger_manager.go:116 (0x106574deb)
        (*requestRecordsManager).handler-fm.(*requestRecordsManager).handler.func1.1: panic(localPanicErr)
/usr/local/go/src/runtime/panic.go:770 (0x104ea8c43)
        gopanic: fn()
/usr/local/go/src/runtime/panic.go:261 (0x104ec3d27)
        panicmem: panic(memoryError)
/usr/local/go/src/runtime/signal_unix.go:881 (0x104ec3cf4)
        sigpanic: panicmem()
/Users/hezi/Documents/repos/hub/models/git/commit_status.go:544 (0x105f40444)
        CommitStatusesHideActionsURL: if status.Repo == nil {
/Users/hezi/Documents/repos/hub/routers/web/repo/repo.go:669 (0x106e44397)
        SearchRepo: git_model.CommitStatusesHideActionsURL(ctx, latestCommitStatuses)
/usr/local/go/src/reflect/value.go:596 (0x104f629ef)
        Value.call: call(frametype, fn, stackArgs, uint32(frametype.Size()), uint32(abid.retOffset), uint32(frameSize), &regArgs)
/usr/local/go/src/reflect/value.go:380 (0x104f61e93)
        Value.Call: return v.call("Call", in)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:172 (0x10657dd67)
        toHandlerProvider.func1.1: ret := fn.Call(argsIn)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:182 (0x10657ddd7)
        toHandlerProvider.func1.1: next.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/go/pkg/mod/github.com/go-chi/chi/[email protected]/chain.go:31 (0x1065758bf)
        (*ChainHandler).ServeHTTP: c.chain.ServeHTTP(w, r)
/Users/hezi/go/pkg/mod/github.com/go-chi/chi/[email protected]/mux.go:459 (0x106578607)
        (*Mux).routeHTTP: h.ServeHTTP(w, r)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:182 (0x10657ddd7)
        toHandlerProvider.func1.1: next.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:182 (0x10657ddd7)
        toHandlerProvider.func1.1: next.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:182 (0x10657ddd7)
        toHandlerProvider.func1.1: next.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/go/pkg/mod/github.com/go-chi/chi/[email protected]/middleware/get_head.go:37 (0x106dafb3b)
        GetHead.func1: next.ServeHTTP(w, r)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:136 (0x10657e1a7)
        toHandlerProvider.wrapHandlerProvider[...].func2.1: h.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:182 (0x10657ddd7)
        toHandlerProvider.func1.1: next.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/services/context/context.go:218 (0x106612b6b)
        Contexter.func1.1: next.ServeHTTP(ctx.Resp, ctx.Req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:136 (0x10657e1a7)
        toHandlerProvider.wrapHandlerProvider[...].func2.1: h.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/go/pkg/mod/gitea.com/go-chi/[email protected]/session.go:257 (0x1065265a3)
        Sessioner.func1.1: next.ServeHTTP(w, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:136 (0x10657e1a7)
        toHandlerProvider.wrapHandlerProvider[...].func2.1: h.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/go/pkg/mod/github.com/go-chi/chi/[email protected]/mux.go:73 (0x106576413)
        (*Mux).ServeHTTP: mx.handler.ServeHTTP(w, r)
/Users/hezi/go/pkg/mod/github.com/go-chi/chi/[email protected]/mux.go:327 (0x106577c5f)
        (*Mux).Mount.func1: handler.ServeHTTP(w, r)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/go/pkg/mod/github.com/go-chi/chi/[email protected]/mux.go:459 (0x106578607)
        (*Mux).routeHTTP: h.ServeHTTP(w, r)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/go/pkg/mod/github.com/go-chi/chi/[email protected]/mux.go:73 (0x106576413)
        (*Mux).ServeHTTP: mx.handler.ServeHTTP(w, r)
/Users/hezi/go/pkg/mod/github.com/go-chi/chi/[email protected]/mux.go:327 (0x106577c5f)
        (*Mux).Mount.func1: handler.ServeHTTP(w, r)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/go/pkg/mod/github.com/go-chi/chi/[email protected]/mux.go:459 (0x106578607)
        (*Mux).routeHTTP: h.ServeHTTP(w, r)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/routing/logger_manager.go:122 (0x106574c87)
        (*requestRecordsManager).handler-fm.(*requestRecordsManager).handler.func1: next.ServeHTTP(w, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:136 (0x10657e1a7)
        toHandlerProvider.wrapHandlerProvider[...].func2.1: h.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/go/pkg/mod/github.com/chi-middleware/[email protected]/middleware.go:37 (0x106cf6cd7)
        ProtocolMiddlewares.ForwardedHeaders.func4.1: h.ServeHTTP(w, r)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:136 (0x10657e1a7)
        toHandlerProvider.wrapHandlerProvider[...].func2.1: h.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/routers/common/middleware.go:59 (0x106cf7ccb)
        ProtocolMiddlewares.func3.1: next.ServeHTTP(context.WrapResponseWriter(resp), req.WithContext(cache.WithCacheContext(ctx)))
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:136 (0x10657e1a7)
        toHandlerProvider.wrapHandlerProvider[...].func2.1: h.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/routers/common/middleware.go:50 (0x106cf78e3)
        ProtocolMiddlewares.func2.1: next.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:136 (0x10657e1a7)
        toHandlerProvider.wrapHandlerProvider[...].func2.1: h.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/routers/common/middleware.go:36 (0x106cf75ab)
        ProtocolMiddlewares.func1.1: next.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/Documents/repos/hub/modules/web/handler.go:136 (0x10657e1a7)
        toHandlerProvider.wrapHandlerProvider[...].func2.1: h.ServeHTTP(resp, req)
/usr/local/go/src/net/http/server.go:2166 (0x1053089b7)
        HandlerFunc.ServeHTTP: f(w, r)
/Users/hezi/go/pkg/mod/github.com/go-chi/chi/[email protected]/mux.go:90 (0x1065763cf)
        (*Mux).ServeHTTP: mx.handler.ServeHTTP(w, r)
/Users/hezi/Documents/repos/hub/modules/web/route.go:225 (0x10657f937)
        (*Router).normalizeRequestPath: next.ServeHTTP(resp, req)
/Users/hezi/Documents/repos/hub/modules/web/route.go:165 (0x10657f4ff)
        (*Router).ServeHTTP: r.normalizeRequestPath(w, req, r.chiRouter)
/usr/local/go/src/net/http/server.go:3137 (0x10530bdcb)
        serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/usr/local/go/src/net/http/server.go:2039 (0x105307587)
        (*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/usr/local/go/src/runtime/asm_arm64.s:1222 (0x104ee67a3)
        goexit: MOVD    R0, R0  // NOP

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we are getting nil elements in the latestCommitStatuses slice (i.e. because of this call).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nil needs to be cache which means there is no commit status.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#31731

@yp05327 yp05327 mentioned this pull request Jul 30, 2024
Merged
lunny pushed a commit that referenced this pull request Jul 30, 2024
@yp05327
Fix Null Pointer error for CommitStatusesHideActionsURL (#31731)
0a11bce 
Fix #30156 (comment)

Forgot fixing it in #31719
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request Aug 6, 2024
@yp05327 @earl-warren
Fix Null Pointer error for CommitStatusesHideActionsURL (#31731)
49eb831 
Fix go-gitea/gitea#30156 (comment)

Forgot fixing it in #31719

(cherry picked from commit 0a11bce87f07233d5f02554b8f3b4a2aabd37769)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lunny lunny lunny approved these changes

@yp05327 yp05327 yp05327 left review comments

@badhezi badhezi badhezi left review comments

@kemzeb kemzeb kemzeb left review comments

@wolfogre wolfogre wolfogre approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/gitea-actions related to the actions of Gitea type/bug
Projects
None yet
Milestone
1.23.0
Development

Successfully merging this pull request may close these issues.

User with no action unit access permission can also see the commit status
9 participants
@Zettat123 @yp05327 @wxiaoguang @lunny @wolfogre @badhezi @kemzeb @silverwind @GiteaBot