Allow Override PR History link #24377
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
/cc @BenTheElder |
k8s-ci-robot
added
size/M
k8s-ci-robot
added
the
approved
| return "", fmt.Errorf("failed compiling template %q: %v", PRHistLinkTemplate, err) | ||
| } | ||
| tmpBuff := bytes.Buffer{} | ||
| if err = tmp.Execute(&tmpBuff, struct { |
There was a problem hiding this comment.
Trying to think of any vulnerabilities... Looks vulnerable to XSS. For example, what happens if I use a template like: javascript:doSomethingEvil()
... I think "sanitizing/validating" a template would be tricky because of HTML entities & URL encoding.
I forget if Spyglass configuration is considered "safe" (requiring PR reviews, plus or minus in-repo config), but I'm assuming two bad actors could collaborate to merge it??
There was a problem hiding this comment.
Technically this is possible. But this field can only be changed in config.yaml file, which is supposed to be guarded by OWNERS file, to ensure that changes here can only be merged if a prow admin approve the change. So we might be fine?
There was a problem hiding this comment.
It's possible that a (new/distracted/etc) OWNER might not pay full attention to it.
I'll leave the final say to you/others.
There was a problem hiding this comment.
Prow had several other fields like https://github.com/kubernetes/test-infra/pull/24377/files#diff-88e6b42a331a7aa9260f9a830961e3ba3c77e0e6d781465378b510d7abc1517cR997 could also be exposed to this kind of problem. Generally this should be easy to identify during code review
PR history link /pr-history from deck is currently only supported for GitHub repos, which turned out to be internal error for gerrit based prow. Allow overriding the template could possibly let users pointing to etc
chaodaiG
force-pushed
the
pr-history-link
branch
from
2afd8d2 to
4a4a084
Compare
|
/cc @alvaroaleman |
|
@alvaroaleman , mind review? |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, chaodaiG The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
PR history link /pr-history from deck is currently only supported for GitHub repos, which turned out to be internal error for gerrit based prow. Allow overriding the template could possibly let users pointing to
/repo=<REPO>&pull=<PULL>etc