Skip to content

setopt: clear proxy auth properly on NULL#21696

Closed
bagder wants to merge 2 commits into
masterfrom
bagder/proxy-auth
Closed

setopt: clear proxy auth properly on NULL#21696
bagder wants to merge 2 commits into
masterfrom
bagder/proxy-auth

Conversation

@bagder

@bagder bagder commented May 20, 2026

Copy link
Copy Markdown
Member

No description provided.

@github-actions github-actions Bot added the tests label May 20, 2026
Comment thread tests/libtest/lib1648.c Dismissed
Comment thread tests/libtest/lib1648.c Dismissed
@bagder bagder marked this pull request as ready for review May 20, 2026 12:47
@bagder bagder closed this in 88c7e16 May 20, 2026
@bagder bagder deleted the bagder/proxy-auth branch May 20, 2026 13:55
@jtakalai jtakalai mentioned this pull request May 21, 2026
@bagder

bagder commented May 21, 2026

Copy link
Copy Markdown
Member Author

@vszakats this silly bot appears because we use CodeQL. Why do we use CodeQL again? I can't recall it doing much good for us.

@vszakats

vszakats commented May 21, 2026

Copy link
Copy Markdown
Member

@vszakats this silly bot appears because we use CodeQL. Why do we use CodeQL again? I can't recall it doing much good for us.

It's suggested by GitHub folks, as one the five pillars of security tools.

It's indeed not catching much issues and have in rare cases false
positives (last time in December and before that this same HTTPS
warning in November). IMO this mirrors the experience with other
static analyzers we use. I tried making these builds useful as mere
build tests too. (The past downside is no longer an issue after
disabling the so called TRAP cache.)

Now Python is also analyzed via the Code Quality feature (it looks
parallel, though that's actually finding stuff, and I guess we haven't
much looked at it yet: https://github.com/curl/curl/security/quality).
GHA is covered by zizmor, and C is probably somewhat redundant
with clang-tidy, gcc analyzer and the rest.

I'd personally probably keep these for redundancy, but if the false
positives are too annoying, we may also disable some or all of these
CodeQL jobs. I don't feel strongly to keep them at all cost.

@bagder

bagder commented May 21, 2026

Copy link
Copy Markdown
Member Author

It's suggested by GitHub folks, as one the five pillars of security tools.

Sure, but it's their tool so they're not really impartial in the matter. To me their voice on this is less interesting.

But thanks for the details. I don't think it's too annoying like at this level so I'm not going to push for removal either at this point. Let's see how things develop going forward.

There is apparently a SARIF filter feature we can use to exclude for example test code.

@vszakats

Copy link
Copy Markdown
Member

This filter looks useful to me also. I'd hesitate adding a 3rd-party action for this, and I wonder if there is a jq command-line or local script that could do the job. Will look later into what the Action os doing. Another option is to just not build tests here, with downside that the build will be missing and coverage % falls.

outcast36 pushed a commit to greearb/curl that referenced this pull request Jun 3, 2026
Verify NULLed proxy credentials with test1648

Closes curl#21696
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants