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.

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

GitHub API Deprecations (repoManager and GH import pending failure) #1705

Open
Martii opened this issue Feb 5, 2020 · 15 comments
Open

GitHub API Deprecations (repoManager and GH import pending failure) #1705

Martii opened this issue Feb 5, 2020 · 15 comments
Labels
CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. dependency issue Hmmph! A dependency issue expedite Immediate and on the front burner. migration Use this to indicate that it may apply to an existing or announced migration.
Milestone

Comments

@Martii
Copy link
Member

Martii commented Feb 5, 2020

Got this in the email today (which is a little weird since I don't have OUJS GH access... but none-the-less):

Hello there!

On February 4th, 2020 at 13:20 (UTC) your application (OpenUserJS) used its client_id and client_secret (with the User-Agent NodeJS HTTP Client) as part of a set of query parameters to access an endpoint through the GitHub API:

https://api.github.com/user/_useridclipped_

Please use Basic Authentication instead as using OAuth credentials in query parameters has been deprecated.

Depending on your API usage, we'll be sending you this email reminder at most once every 3 days.
Just one URL that was accessed with a User-Agent combination will be listed in the email reminder, not all.

Visit https://developer.github.com/changes/2019-11-05-deprecated-passwords-and-authorizations-api/#authenticating-using-query-parameters for more information.

Thanks,
The GitHub Team

Target code point is probably https://github.com/OpenUserJS/OpenUserJS.org/blob/1261d16/libs/repoManager.js#L67.

I don't have much time to work on this but it seems that GH importing will fail eventually when it is EOL'd after deprecation. It is open for assignment atm.

See also:

@sizzlemctwizzle
Seems like you will need to set up something with OAuth if I'm skimming this correctly.

For everyone else when/if it breaks, it breaks... the work around should be to paste your source into the browser from github raw url. It would be nice to have this OUJS feature continued as a lot of people do use it. As far as my past changes to the webhook this theoretically shouldn't change anything there. i.e. if your OUJS account has GH authentication then the webhook should still process push notices.

Cc: @Zren

@Martii Martii added sooner Sooner would be appreciated. migration Use this to indicate that it may apply to an existing or announced migration. CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. labels Feb 5, 2020
@Martii
Copy link
Member Author

Martii commented Feb 5, 2020

Hmmm. Thinking out loud for a moment:

Re:

OAuth2 token (sent in a header)

curl -H "Authorization: token OAUTH-TOKEN" https://api.github.com

Note: GitHub recommends sending OAuth tokens using the Authorization header.

Read more about OAuth2. Note that OAuth2 tokens can be acquired using the web application flow for production applications.

Ignoring "Basic Authentication" for the moment which seems unclear... if the Header is sent that might work? EDIT But whose TOKEN... OUJS's or the user that is logged in (which I doubt the latter can be accessed but still investigating).

See also:

  1. Use the access token to access the API

The access token allows you to make requests to the API on a behalf of a user.

Authorization: token OAUTH-TOKEN
GET https://api.github.com/user

For example, in curl you can set the Authorization header like this:

curl -H "Authorization: token OAUTH-TOKEN" https://api.github.com/user

Ref(s):

@Martii
Copy link
Member Author

Martii commented Feb 5, 2020

LOL *eyeroll*

Hi @Martii,

You recently used a password to access an endpoint through the GitHub API using curl/7.58.0. On July 1st, 2020, basic authentication using password to this endpoint will no longer work:

https://api.github.com/

We recommend using a personal access token (PAT) with the appropriate scope to access this endpoint instead. Visit https://github.com/settings/tokens for more information.

Thanks,
The GitHub Team

Tried out their "Basic Authentication" and got this email from them. WT*beep*.

From some more skimming it seems that the authentication is needed to change the Rate Limiting on their end. May have to do this anonymously if possible... but could pose some additional problems.

@Martii Martii self-assigned this Feb 6, 2020
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Feb 6, 2020
* Too many nested if/else's makes it highly unreadable between OAuth and OpenID i.e. easier commit history searching. Been thinking about doing this since removal commit... doing it.
* Probable location to add `aToken` to  which is actually `aAccessToken` in dep README's. Possible location could be `aReq.session.accessToken` Applies to OpenUserJS#1705

Post OpenUserJS#889 OpenUserJS#564
Martii added a commit that referenced this issue Feb 6, 2020
* Too many nested if/else's makes it highly unreadable between OAuth and OpenID i.e. easier commit history searching. Been thinking about doing this since removal commit... doing it.
* Probable location to add `aToken` to  which is actually `aAccessToken` in dep README's. Possible location could be `aReq.session.accessToken` Applies to #1705

Post #889 #564

Auto-merge
@Martii Martii removed their assignment Feb 6, 2020
@Martii
Copy link
Member Author

Martii commented Feb 7, 2020

Depending on your API usage, we'll be sending you this email reminder at most once every 3 days.

This part of GH is making it really difficult to debug. I got two today but not sure when it happened because GH's clocks are way off compared to when the events were actually initiated (NIST coordinated here but the messages are either earlier or later than what I tried). So I get to wait another day or more (up to three since they haven't quite got the reminder notice correct on max). There also seems to be some sort of API caching going on because I tried a third test and nothing in a trace and nothing in email yet.

I'm still not entirely sure if the "should" for the webhook will be a "won't".

Options:

  1. If it is for sure the import routine failing we might be able to send the accessToken (aToken to be specific) in it's place. This is the least complicated for the end user rather than creating a personal access token but untested atm. OUJS might be able to have it's own token but that may prove to have GH Rate Limiting if we have a ton of users sync at the same time (e.g. they'll detect our token as a DoS attack and limit (if not kill) retrieval). EDIT Currently this is synonymous with the behaviour and usage of QSP's

  2. If it's the webhook that is pending failure then we'll need to allow each user to store a personal access token because they won't always be logged into OUJS. If we utilize a stored personal access token only the single user gets rate limited. I'm personally leaning towards this although it does compromise some anonymity/privacy but passes DoS responsibility onto the end user.

Total mess having them deprecate the QSP's imho atm.


Misc note(s):

  • client_id and client_secret, in this context, is the OUJS (Auth Strategy API Keys) id and key in UI.
  • fetchRaw is normally unauthenticated and may have a stricter GH rate limit since it's "anonymous" access.
  • fetchRaw is currently unauthenticated ... reverse trace (manual):
    • controllers/scriptStorage.js
      • webhook()
      • RepoManager.getManager() (instance)
      • repoManager.loadScripts() (call method)
    • libs/repoManager.js
      • loadScripts() (prior call method)
      • repoManager.makeRepoArray() (call method)
      • fetchRaw() (call method and no fetchJSON call method appending OUJS client_id and client_secret... probable not webhook related with main issue)
    • controllers/user.js
      • Extension of GH API v3 using custom functions

Martii added a commit to Martii/OpenUserJS.org that referenced this issue Feb 13, 2020
* Please read their CHANGELOGs
* Delete op retested
* *request* is deprecated and another suitable alternative needs to be found. See request/request/issues/3143 and request/request/issues/3142. Note we didn't get the deprecation warning in dev but examined with CHANGELOGs/commits on this package.
  * Affected code *(a lot)*:
    * app.js *(TLS certficate ping)*
    * libs/githubClient.js *(github import/browse)* Related to OpenUserJS#1705
    * libs/repoManager *(github import/browse)* Related to OpenUserJS#1705
    * controllers/scriptStorage.js *(webhook hooks)*
Martii added a commit that referenced this issue Feb 13, 2020
* Please read their CHANGELOGs
* Delete op retested
* *request* is deprecated and another suitable alternative needs to be found. See request/request/issues/3143 and request/request/issues/3142. Note we didn't get the deprecation warning in dev but examined with CHANGELOGs/commits on this package.
  * Affected code *(a lot)*:
    * app.js *(TLS certficate ping)*
    * libs/githubClient.js *(github import/browse)* Related to #1705
    * libs/repoManager *(github import/browse)* Related to #1705
    * controllers/scriptStorage.js *(webhook hooks)*

Auto-merge
joeytwiddle added a commit to joeytwiddle/OpenUserJS.org that referenced this issue Aug 9, 2020
joeytwiddle added a commit to joeytwiddle/OpenUserJS.org that referenced this issue Aug 9, 2020
joeytwiddle added a commit to joeytwiddle/OpenUserJS.org that referenced this issue Aug 9, 2020
Martii pushed a commit that referenced this issue Aug 16, 2020
I don't know if this will work, since I don't have a full test setup.

However this is how we did Basic authentication in our app's fetch requests. So fingers crossed it will work for GitHub too!

Addresses issue #1705

Auto-merge and THANKS!
@Martii
Copy link
Member Author

Martii commented Aug 16, 2020

Keeping this open for a bit in case more reports come through.

@Martii
Copy link
Member Author

Martii commented Nov 7, 2020

Reports have come through and are from #1432 starting at lines 714 forward:

        if (this.auth) {
            var basic;
            switch (this.auth.type) {
                case "oauth":
                    if (this.auth.token) {
                        path += (path.indexOf("?") === -1 ? "?" : "&") +
                            "access_token=" + encodeURIComponent(this.auth.token);
                    } else {
                        path += (path.indexOf("?") === -1 ? "?" : "&") +
                            "client_id=" + encodeURIComponent(this.auth.key) +
                            "&client_secret=" + encodeURIComponent(this.auth.secret);
                    }
                    break;

So migration will need to occur soon, if possible. A fork of the fixed version may be needed in the short term... presuming it's available.


Ref(s):

@Martii Martii added dependency issue Hmmph! A dependency issue expedite Immediate and on the front burner. and removed sooner Sooner would be appreciated. labels Nov 7, 2020
Martii added a commit to Martii/UserScripts that referenced this issue Apr 16, 2021
@Martii
Copy link
Member Author

Martii commented Apr 16, 2021

Okay... so webhook doesn't appear to be directly coupled with github dependency authorization... since most recent webhook test commit reference worked after I forced the rate limit (30 times again on production this time) on the repoManager. (poor mans testing btw ;)


So in short importing will be broken (but I put in a kill switch above) until this can get resolved properly. Buys a little bit of time by losing the import feature but at least it's not everything so far.


I think I'm going to do some random "brown out"s similar, but longer and very random, to what GH did at their blog post to simulate if any unforseen bugs appear... so be aware.

Martii added a commit to Martii/OpenUserJS.org that referenced this issue Apr 16, 2021
* Disables *github* dep authorization which in turn disable importing. When activated use paste to site via Write Script Online or file upload via Upload Script until the dep can be successfully migrated. If authorization is enabled and there's a dep issue a 500 Server error could occur if GH changes something.
* More bookmarks... can't quite tell if it's Fx but the bookmarks are shooting low on auto browser scroll... will try another browser at a future date.
* Some verbiage clarity.

Applies to OpenUserJS#1705
Martii added a commit that referenced this issue Apr 16, 2021
* Disables *github* dep authorization which in turn disable importing. When activated use paste to site via Write Script Online or file upload via Upload Script until the dep can be successfully migrated. If authorization is enabled and there's a dep issue a 500 Server error could occur if GH changes something.
* More bookmarks... can't quite tell if it's Fx but the bookmarks are shooting low on auto browser scroll... will try another browser at a future date.
* Some verbiage clarity.

Applies to #1705

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Apr 17, 2021
* Translate GH's 403 to 503
* If/when the dep gets updated, gracefully handle GH's 403 as much as possible at authenticated and non-authenticated requests.
* The *@octokit/auth-token* dep returns lower casings for `authentication` and `basic`... not sure if matching this makes a difference but today I've been getting 45 maximum on unauthenticated GH rate limiting with this. Post OpenUserJS#1729

Applies to OpenUserJS#1705 OpenUserJS#37
Martii added a commit that referenced this issue Apr 17, 2021
* Translate GH's 403 to 503
* If/when the dep gets updated, gracefully handle GH's 403 as much as possible at authenticated and non-authenticated requests.
* The *@octokit/auth-token* dep returns lower casings for `authentication` and `basic`... not sure if matching this makes a difference but today I've been getting 45 maximum on unauthenticated GH rate limiting with this. Post #1729

Applies to #1705 #37

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Apr 17, 2021
* Authentication confirmation doesn't happen at all on object construction. This routine is instead used to store the values for later usage. A call to a URL will show if there is a higher rate limit. In the newer dep `authorization` is calculated as well.
* Clean up and add existence checks in repoManager. Don't necessarily have to terminate the app if API Keys for GH aren't there... should just use lower rate limit... although other features might depend on those values. Appears to be part of the webhook but not confirmed.

Applies to OpenUserJS#1705 OpenUserJS#37  and post OpenUserJS#1729
Martii added a commit that referenced this issue Apr 17, 2021
* Authentication confirmation doesn't happen at all on object construction. This routine is instead used to store the values for later usage. A call to a URL will show if there is a higher rate limit. In the newer dep `authorization` is calculated as well.
* Clean up and add existence checks in repoManager. Don't necessarily have to terminate the app if API Keys for GH aren't there... should just use lower rate limit... although other features might depend on those values. Appears to be part of the webhook but not confirmed.

Applies to #1705 #37  and post #1729

Auto-merge
@Martii
Copy link
Member Author

Martii commented Apr 17, 2021

Dead code found at:

$ rgrep fetchRecentRepos
libs/repoManager.js:// RepoManager.prototype.fetchRecentRepos = function (aCallback) {

GRR!


This also means the entire Repo object is dead code too. This is why #1729 had no effect... because part of that is dead code too with fetchJSON.

DOUBLE GRR!


rgrep is my alias to recursively search excluding node_modules btw.

Martii added a commit to Martii/OpenUserJS.org that referenced this issue Apr 17, 2021
* Also at some point *node* changed the way to detect if debugging was in process. Updated original comment at OpenUserJS#429 (comment) Been a while since I've been in this mode for deep debug examination.
* Fix missing custom headers for UA. This is optional but didn't know this until now that it had that option for the *github* dep. Post OpenUserJS#1753

Applies to OpenUserJS#1705

Ref(s):
* https://nodejs.org/api/inspector.html#inspector_inspector_url
* https://nodejs.org/api/debugger.html
Martii added a commit that referenced this issue Apr 17, 2021
* Also at some point *node* changed the way to detect if debugging was in process. Updated original comment at #429 (comment) Been a while since I've been in this mode for deep debug examination.
* Fix missing custom headers for UA. This is optional but didn't know this until now that it had that option for the *github* dep. Post #1753

Applies to #1705

Ref(s):
* https://nodejs.org/api/inspector.html#inspector_inspector_url
* https://nodejs.org/api/debugger.html

Auto-merge
@Martii
Copy link
Member Author

Martii commented Apr 17, 2021

Misc note for anyone wanting to follow along... or better yet add your 2 cents... trials and tribulations at https://github.com/octokit/rest.js/discussions/2070 octokit/octokit.js#2070

I'm bullet proofing the github dep and site as much as I can but I have no idea what GH in the back end is going to do to this dep... hopefully it doesn't throw an unexpected error and cause a bazillion restarts... but we'll know soon. 😸

I will definitely be removing the dead code but will probably do that at/near EOL of the github dep.... when they disable QSPs.

@Martii
Copy link
Member Author

Martii commented Apr 19, 2021

Hmmm... seems that https://github.com/octokit/rest.js/discussions has been disabled today. :\


Ahhh I see, I think, they went through a GH name change... new location is https://github.com/octokit/octokit.js/discussions


NOTE(s):

Martii added a commit to Martii/OpenUserJS.org that referenced this issue Apr 21, 2021
* Transform GH 401 to 503 .  Mentioned at https://developer.github.com/changes/2020-02-10-deprecating-auth-through-query-param/ and encountered with newer migrated dep.
* Spec for authorization shows uppercasing... so new dep has it with incorrect casing I believe... may not matter on their server but consistancy here. Reverting.
* Add a few more important comments

Applies to OpenUserJS#1705 OpenUserJS#37 and post OpenUserJS#1799

Ref(s):
* https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.8
* https://developer.mozilla.org/docs/Web/HTTP/Authentication
Martii added a commit that referenced this issue Apr 21, 2021
* Transform GH 401 to 503 .  Mentioned at https://developer.github.com/changes/2020-02-10-deprecating-auth-through-query-param/ and encountered with newer *(attempted)* migrated dep.
* Spec for authorization shows uppercasing... so new dep has it with incorrect casing I believe... may not matter on their server but consistancy here. Reverting.
* Add a few more important comments

Applies to #1705 #37 and post #1799

Ref(s):
* https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.8
* https://developer.mozilla.org/docs/Web/HTTP/Authentication

Auto-merge
@Martii
Copy link
Member Author

Martii commented Apr 21, 2021

Interesting... blog post changed:

Brownouts

During a brownout, authentication using query parameters will temporarily fail. The goal is to trigger alerts (assuming there are any) on our customers' services to help them find unmigrated authentication calls.

The brownouts are scheduled for:

  • May 5, 2021: For 12 hours starting at 14:00 UTC

  • June 9, 2021: For 24 hours starting at 14:00 UTC

  • August 11, 2021: For 48 hours starting at 14:00 UTC

Removal date

All authentication using query parameters will return a status code of 401 like all other auth failures starting on:

  • September 8 2021 at 14:00 UTC

... modifying milestone again.


Good day now, May 5th, to test these changes and how it will affect us. 😸

Martii added a commit to Martii/UserScripts that referenced this issue May 5, 2021
@Martii
Copy link
Member Author

Martii commented May 5, 2021

Test forced rate limit on dev... Wasn't totally expecting a 403 because docs says 401 but close enough (kind of figured this with #1799 and the vague wording that Sept 8, 2021 is probably going to be the actual failure point with 401 ):

(NOTE: Some sanitized and beautified output)

$ node inspect app.js
...
C
...
REQUEST:  {
  host: 'api.github.com',
  port: 443,
  path: '/users/Martii?client_id={client_id}',
  method: 'get',
  headers: {
    host: 'api.github.com',
    'content-length': '0',
    'user-agent': 'OpenUserJS/0.5.2 (Linux; x64; rv:249e16f) OUJS/20131106 OpenUserJS.org/249e16f',
    accept: 'application/vnd.github.v3+json'
  }
}
STATUS: 403
HEADERS: {
  "server": "GitHub.com",
  "date": "Wed, 05 May 2021 19:34:34 GMT",
  "content-type": "application/json; charset=utf-8",
  "content-length": "276",
  "x-ratelimit-limit": "60",
  "x-ratelimit-remaining": "0",
  "x-ratelimit-reset": "1620246698",
  "x-ratelimit-used": "60",
  "x-ratelimit-resource": "core",
  "x-github-media-type": "github.v3; format=json",
  "access-control-expose-headers": "ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset",
  "access-control-allow-origin": "*",
  "strict-transport-security": "max-age=31536000; includeSubdomains; preload",
  "x-frame-options": "deny",
  "x-content-type-options": "nosniff",
  "x-xss-protection": "0",
  "referrer-policy": "origin-when-cross-origin, strict-origin-when-cross-origin",
  "content-security-policy": "default-src 'none'",
  "vary": "Accept-Encoding, Accept, X-Requested-With",
  "x-github-request-id": "{X-GITHUB-REQUEST-ID}",
  "connection": "close"
}
[error] [Error: {"message":"API rate limit exceeded for {IP}. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)","documentation_url":"https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting"}] {
[error]   code: 403
[error] } null Martii
API rate limit exceeded for {IP}. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)
...

SUMMARY

Dev test (started manually)

  1. Started up normally.
  2. Import still worked during lower rate limit in all three tested routes.
  3. When rate limit exceeded gave 403 in all three routes back-end and 503 front-end.
  4. Rate limit is still halved. i.e. two requests are happening currently per page.

Pro test (already running)

  1. Running normally.
  2. Import still worked during lower rate limit in all three tested routes.
  3. When rate limit exceeded gave 403 in all three routes back-end and 503 front-end.
  4. Rate limit is still halved. i.e. two requests are happening currently per page.
  5. Webhook route still works. YAY!

Pro test (restarted ~20:02 UTC)

  1. Running normally.
  2. Import still worked during lower rate limit in all three tested routes.
  3. When rate limit exceeded gave 403 in all three routes back-end and 503 front-end.
  4. Rate limit is still halved. i.e. two requests are happening currently per page.

NOTE(S):

  • I would have used "502 Bad Gateway" but per wikipedia that usually means:

The server was acting as a gateway or proxy and received an invalid response from the upstream server.

... whereas "503 Service Unavailable" usually means:

The server (OUJS) cannot handle the request (because it is overloaded (GH) or down for maintenance). Generally, this is a temporary state.

... is more accurate... plus I think we've had enough of non-descript status codes (still) from the USO days.

  • Same results for modified test:
var Octokit = require("@octokit/rest").Octokit;

var clientId = 'Can not post this publicly but it is accurate';
var clientSecret = 'Can not post this publicly but it is accurate';

var oauthToken = Buffer.from([`${clientId}`, `${clientSecret}`].join(':')).toString('base64');

var test = new Octokit({
  auth: `basic ${oauthToken}`
});

test.paginate("GET /users/{username}/repos", { username: "Martii" }).then(repositories => {
  console.log("%d repositories found", repositories.length)
});

RESULT:

...
(node:48411) UnhandledPromiseRejectionWarning: HttpError: Bad credentials
    at ~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/@octokit/request/dist-node/index.js:68:23
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async Object.next (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/@octokit/plugin-paginate-rest/dist-node/index.js:62:26)
...

@Martii
Copy link
Member Author

Martii commented May 5, 2021

For a note with #1802 #1799, this is what should be showing for #37 compliance with GH rate limit (GH RL) exceeded:

Since we don't currently know if GH RL is rolling using the message ~"...back later".


Loosely related at jaredhanson/passport-github#75 followup is needed... we don't currently nab emails but still might be an issue at some point down the line.

Martii added a commit to Martii/OpenUserJS.org that referenced this issue May 19, 2021
* I'm guessing the failure that was encountered at OpenUserJS#1705 (comment) may have been a fluke on the GH back-end.
* GH Docs and blog posts are a little too vague for my tastes with `username` and `password`... however upon manual deconstruction and inspection of the *github* dep at https://github.com/OpenUserJS/rest.js/blob/29dedb3022649c27ac1dad79326270b6b2a48047/index.js#L730 it uses nearly what the maintainer mentioned at [the discussion](octokit/octokit.js#2070) and what joeytwiddle did at OpenUserJS#1729 in the recently discovered dead code. *nodes* default for `Buffer` is `utf8` whereas *github*@0.2.4 via commit ref uses `ascii`. Since hashes usually only contain ASCII should not be an issue.
* Newest dep at the time of testing with *@octokit/rest* for this seems to be unstable so going with this for a while since it currently gives the higher rate limit. This could change depending on what they do in the back-end.

Applies to OpenUserJS#1705

NOTE(s)
* a GH url changed recently from https://github.com/octokit/rest.js to https://github.com/octokit/octokit.js ... may affect things even further.
@Martii Martii mentioned this issue May 19, 2021
Martii added a commit that referenced this issue May 19, 2021
* I'm guessing the failure that was encountered at #1705 (comment) may have been a fluke on the GH back-end.
* GH Docs and blog posts are a little too vague for my tastes with `username` and `password`... however upon manual deconstruction and inspection of the *github* dep at https://github.com/OpenUserJS/rest.js/blob/29dedb3022649c27ac1dad79326270b6b2a48047/index.js#L730 it uses nearly what the maintainer mentioned at [the discussion](octokit/octokit.js#2070) and what joeytwiddle did at #1729 in the recently discovered dead code. *nodes* default for `Buffer` is `utf8` whereas *github*@0.2.4 via commit ref uses `ascii`. Since hashes usually only contain ASCII should not be an issue.
* Newest dep at the time of testing with *@octokit/rest* for this seems to be unstable so going with this for a while since it currently gives the higher rate limit. This could change depending on what they do in the back-end.

Applies to #1705

NOTE(s)
* a GH url changed recently from https://github.com/octokit/rest.js to https://github.com/octokit/octokit.js ... may affect things even further.

Auto-merge
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Sep 2, 2021
* This finally warns using *npm* under `engines` with *node*
* Also startup with strict instead of lax. Has been in place for about a month as a test for login issues. Still `lax` is needed for OAuth/etc. unfortunately.
* Delete op, sanitize, markdown, moderation flagging, overall UI... retested
* Move git repo linkage in package.json to https. Seems like https://github.blog/2021-09-01-improving-git-protocol-security-github/#how-do-i-prepare with "anonymous" git might be affecting this from what I gather. See also https://docs.npmjs.com/cli/v7/configuring-npm/package-json#git-urls-as-dependencies . Works on current dev stack... haven't ever tried this on pro... so \*crosses things\* Indirectly applies to OpenUserJS#1705 . After March 15, 2022 can retry `git:` but preventative atm.
@Martii Martii mentioned this issue Sep 2, 2021
Martii added a commit that referenced this issue Sep 2, 2021
*node* bump

* This finally warns using *npm* under `engines` with *node*
* Also startup with strict instead of lax. Has been in place for about a month as a test for login issues. Still `lax` is needed for OAuth/etc. unfortunately.
* Delete op, sanitize, markdown, moderation flagging, overall UI... retested
* Move git repo linkage in package.json to https. Seems like https://github.blog/2021-09-01-improving-git-protocol-security-github/#how-do-i-prepare with "anonymous" git might be affecting this from what I gather. See also https://docs.npmjs.com/cli/v7/configuring-npm/package-json#git-urls-as-dependencies . Works on current dev stack... haven't ever tried this on pro... so \*crosses things\* Indirectly applies to #1705 . After March 15, 2022 can retry `git:` but preventative atm.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. dependency issue Hmmph! A dependency issue expedite Immediate and on the front burner. migration Use this to indicate that it may apply to an existing or announced migration.
Development

No branches or pull requests

1 participant