git-eca-rest-api issueshttps://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues2024-03-20T13:51:51Zhttps://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/75Should we require CSRF/login to query account lookups2024-03-20T13:51:51ZMartin Lowemartin.lowe@eclipse-foundation.orgShould we require CSRF/login to query account lookupsA new endpoint was recently added to the API which returns a simple response with only a status code to indicate whether an email (indicated by query string parameters) has a signed ECA on file. This check does not return the user object...A new endpoint was recently added to the API which returns a simple response with only a status code to indicate whether an email (indicated by query string parameters) has a signed ECA on file. This check does not return the user object or any information about the user other than if the email indicated in the request is associated with an account that has an ECA/MCCA on file.
Should we add some sort of protective layer to this call to avoid any sort of data mining for emails that have associated Eclipse accounts? While it won't tell you who the account is linked to, it will indicate whether an account exists at least.
/cc @cguindon
/cc @mbarbero - I've added you as well as you may have some inputs on this (just in case, CYA type thing haha)https://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/92Proposal: Consider requiring ECA validation on all commits2024-03-20T13:59:12ZChristopher Guindonchris.guindon@eclipse-foundation.orgProposal: Consider requiring ECA validation on all commitsWe currently only enforce the ECA on an Eclipse Project repo. This is everything under gitlab.eclipse.org/eclipse.
We currently allow contributors to push to their fork without an ECA. At this time, our contributors are only made aware...We currently only enforce the ECA on an Eclipse Project repo. This is everything under gitlab.eclipse.org/eclipse.
We currently allow contributors to push to their fork without an ECA. At this time, our contributors are only made aware of a validation error when they try to merge their changes to the project repo.
In the short term, we are planning to inform the user with a warning on forks. See https://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/84
With this issue, I am proposing that we block contributions if the user does not have an ECA on file when pushing to a non-eclipse project repo.
We did this to make it easy for our users to start contributing on Gitlab but we are now noticing that this is creating some extra confusion with some users when it's time for them to submit a MR and they finally see the ECA error.
We've seen this today via https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/2199#note_1035535
I am thinking it would be best to simply enforce the ECA on all commits. This way, our contributors would be informed the second they try to push something to GitLab that they need to sign our ECA if they are offside.
**Other things to consider:**
- Should we also require the ECA for contributions to project wikis or should we keep our exception for that?
- Should we require the ECA for all contributions made under the eclipse-wg and eclipse-ig group?
- Is there any expectations we should consider?
//cc @malowe @wbeaton @droy @fgurrhttps://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/118Add support for the field website_repos2024-03-20T14:00:55ZMartin Lowemartin.lowe@eclipse-foundation.orgAdd support for the field website_reposTo better support project websites and to reduce the need to mark websites as project code, we should support the website_repos field from PMI. This would only impact ECA validation and not access permissions set by sync scripts, but sho...To better support project websites and to reduce the need to mark websites as project code, we should support the website_repos field from PMI. This would only impact ECA validation and not access permissions set by sync scripts, but should allow for easier management of the websites at least. This came up from discussions had on the following issue: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/2614#note_1076961
The only field that we would care about is the URL of the website repo, as the goal is to just validate what project this belongs to for purposes of bot validation.Martin Lowemartin.lowe@eclipse-foundation.orgMartin Lowemartin.lowe@eclipse-foundation.org2023-02-28https://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/122Remove fingerprint mechanism from ECA validation + endpoints2024-03-20T14:00:58ZMartin Lowemartin.lowe@eclipse-foundation.orgRemove fingerprint mechanism from ECA validation + endpointsWhile initially, the fingerprint mechanism worked for what it was intended, it is causing issues with URLs shifting as PRs are updated which is not user-friendly. This feature was added to get around a bug with the persistence code we us...While initially, the fingerprint mechanism worked for what it was intended, it is causing issues with URLs shifting as PRs are updated which is not user-friendly. This feature was added to get around a bug with the persistence code we use that does not return the IDs of new records when generators are used. To properly resolve this issue, the issue with IDs will need to be solved first.
In place of the fingerprint, a new auto-incrementing id column will be used. This will identify a set of commits, and if passed, some basic validation will be done to ensure it is the same general request before updating. If either the group ID isn't passed or isn't valid, a new record will be generated to identify these requests. This will provide more stable URLs and easier lookups that don't make use of interpreted values that shift depending on content.Martin Lowemartin.lowe@eclipse-foundation.orgMartin Lowemartin.lowe@eclipse-foundation.orghttps://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/127Automatically add GitHub PR review comment telling contributors how to sign ECA2024-03-20T14:01:10ZJonah GrahamAutomatically add GitHub PR review comment telling contributors how to sign ECA## Summary
Can the eclipsefdn/eca bot also add a comment to the PR when it fails telling contributors how to resolve the problem? At the moment the details link has some of that information, but you can't see that info without an accoun...## Summary
Can the eclipsefdn/eca bot also add a comment to the PR when it fails telling contributors how to resolve the problem? At the moment the details link has some of that information, but you can't see that info without an account.
## Steps to reproduce
Create a PR on Eclipse CDT without a valid ECA on file.
## What is the current bug behavior?
This is what appears in, for example, https://github.com/eclipse-cdt/cdt/pull/357
![image](/uploads/03736603d6d4b880e0d0b98fcaa7ef93/image.png)
but clicking on Details leads to:
![image](/uploads/33c5a8d70adf26ffbb841307909567ac/image.png)
## What is the expected correct behavior?
An automatic comment that says something like:
Thank you for your contribution. Unfortunately the Eclipse Foundation ECA tool cannot find your signed ECA. Please complete the ECA at ....
## Relevant logs and/or screenshots
(Add a link to or paste any relevant logs - please use code blocks (```) to format console output, logs, and code, as it's very hard to read otherwise.)
## Priority
- [ ] Urgent
- [ ] High
- [x] Medium
- [ ] Low
## Severity
- [ ] Blocker
- [ ] Major
- [x] Normal
- [ ] Low
## Impact
This is a case where having some automation would help because:
1. The user will benefit because the feedback will be while they are currently looking at the issue, rather than waiting for a committer to get involved.
2. I have to do this on occasion, and I imagine this is repeated across all projects. I have had to do it at least 3 times in the last couple of months.
3. I like the idea of it looking like automatic bureaucracy that the bots enforce and that I don't have to get involved
4. I have rarely (if ever?) seen someone fail ECA check on GitHub and go correct it based on *only* the check failing.
Here is an example of what a user who was more [engaged and asked questions](https://github.com/eclipse-cdt/cdt/pull/273):
![image](/uploads/de13b01e7d7881dd898e7b58136abb68/image.png)https://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/129Duplicate commit status entries created during validation2024-03-20T14:02:36ZMartin Lowemartin.lowe@eclipse-foundation.orgDuplicate commit status entries created during validationWhen investigating an issue, I noticed that there are duplicates for commits that have been submitted multiple times to the API. We need to properly update existing entries as this will lead to a much faster inflation of commit count and...When investigating an issue, I noticed that there are duplicates for commits that have been submitted multiple times to the API. We need to properly update existing entries as this will lead to a much faster inflation of commit count and could impact performance. An example of this in the wild is https://api.eclipse.org/git/eca/status/2d3173d62295ec0d871494c2ad6f2fd8/uihttps://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/138HCaptcha doesn't use script integrity checking for status page2024-03-20T14:17:36ZMartin Lowemartin.lowe@eclipse-foundation.orgHCaptcha doesn't use script integrity checking for status pageThe following discussion from !150 should be addressed:
- [ ] @zacharysabourin started a [discussion](https://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/merge_requests/150#note_1194266): (+2 comments)
> This was a sec...The following discussion from !150 should be addressed:
- [ ] @zacharysabourin started a [discussion](https://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/merge_requests/150#note_1194266): (+2 comments)
> This was a security risk. As per the recommendations on [this page](https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity), I downloaded the file to a temp folder and generated a hash using this command:
>
> ```bash
> cat api.js?hl=en | openssl dgst -sha384 -binary | openssl base64 -A
> ```https://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/140On new GH app installation, revalidate the installation ID cache2024-03-20T14:09:44ZMartin Lowemartin.lowe@eclipse-foundation.orgOn new GH app installation, revalidate the installation ID cacheCurrently, the installation ID cache is rebuilt once an hour by a loading cache to ensure that the IDs stay reasonably up to date. A side effect of this is that any new repositories or installations will not be visible for up to an hour ...Currently, the installation ID cache is rebuilt once an hour by a loading cache to ensure that the IDs stay reasonably up to date. A side effect of this is that any new repositories or installations will not be visible for up to an hour until the cache is rebuilt automatically. To address this issue, a new functionality will be added to the GH webhook callback that will check events related to new installations. These calls will trigger a forced refresh of the cache to pull the new entries to reduce the potential downtime that would otherwise existhttps://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/149Create notification if a PR fails to revalidate after a number of times2024-03-20T14:12:36ZMartin Lowemartin.lowe@eclipse-foundation.orgCreate notification if a PR fails to revalidate after a number of timesWhen attempting to debug a different problem, the logs were overloaded with unrelated errors for PR's that were failing to validate properly. We need to create a mechanism for us to receive notifications when errors are long-standing so ...When attempting to debug a different problem, the logs were overloaded with unrelated errors for PR's that were failing to validate properly. We need to create a mechanism for us to receive notifications when errors are long-standing so that we can address them as they become problematic. I'm thinking we can create a list of addresses that receive the message when a certain threshold is met by the revalidation process.
I'm thinking to start with, I can be the sole person on the list, with a threshold of 100 runs before sending a notification. While that is a lot, that is less than a half day of consistent failures, so it should notify us reasonably fast when there is a broken PR in the system for whatever reason. We can make that threshold configurable as well so that we can fine-tune this in the future easily.
@cguindon any thoughts/concerns about this?Martin Lowemartin.lowe@eclipse-foundation.orgMartin Lowemartin.lowe@eclipse-foundation.orghttps://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/156Update EF project usage to reflect updates *_repos fields2024-03-20T14:17:36ZMartin Lowemartin.lowe@eclipse-foundation.orgUpdate EF project usage to reflect updates *_repos fieldsAs updated in https://gitlab.eclipse.org/eclipsefdn/it/websites/drupal/eclipsefdn/-/merge_requests/265, we should make use of updated projects data that includes non-explicit repository links from the dash project. This should be more ef...As updated in https://gitlab.eclipse.org/eclipsefdn/it/websites/drupal/eclipsefdn/-/merge_requests/265, we should make use of updated projects data that includes non-explicit repository links from the dash project. This should be more efficient than the current logic to match on project repository fields, as well as easier to read.https://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/157Discuss obfuscation of email in public endpoints2024-03-20T14:17:36ZMartin Lowemartin.lowe@eclipse-foundation.orgDiscuss obfuscation of email in public endpointsTo better protect users, we should discuss the option to obfuscate emails for returns and in messages to make it harder to scrape emails from the returns. This would use the same logic as the UI and produce emails like `martin.lowe@eclip...To better protect users, we should discuss the option to obfuscate emails for returns and in messages to make it harder to scrape emails from the returns. This would use the same logic as the UI and produce emails like `martin.lowe@eclipse-f*undation DOT org` which are sufficiently obfuscated to make tooled scraping difficult.https://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/160Validation UI sometimes reporting invalid emails in both success and error se...2024-03-20T14:17:36ZMartin Lowemartin.lowe@eclipse-foundation.orgValidation UI sometimes reporting invalid emails in both success and error sectionsSeen in this [OpenHWGroup validation](https://api.eclipse.org/git/eca/status/gh/openhwgroup/cva5/23), there are some cases where a user can appear in both the valid and invalid section when the user should be in the invalid section.Seen in this [OpenHWGroup validation](https://api.eclipse.org/git/eca/status/gh/openhwgroup/cva5/23), there are some cases where a user can appear in both the valid and invalid section when the user should be in the invalid section.Martin Lowemartin.lowe@eclipse-foundation.orgMartin Lowemartin.lowe@eclipse-foundation.orghttps://gitlab.eclipse.org/eclipsefdn/it/api/git-eca-rest-api/-/issues/161Add an additional webhook event to trigger the eca check2024-03-20T14:17:36ZThomas NeidhartAdd an additional webhook event to trigger the eca checkOne of our projects uses a branch protection rule with a merge queue and has the eca check as required status check defined.
However, in such a case, the eca check is not actually performed as the merge queue sends a specific webhook eve...One of our projects uses a branch protection rule with a merge queue and has the eca check as required status check defined.
However, in such a case, the eca check is not actually performed as the merge queue sends a specific webhook event, see this comment
Please take a look at this ticket: https://github.com/eclipse-apoapsis/.eclipsefdn/issues/3#issuecomment-2007877538
A temporary solution will be to remove the eca check as required status check, but in the long term it would be great to support also such cases.