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

The argocd server api-content-type flag does not allow empty content-type header #17057

Closed
Nibiii opened this issue Jan 31, 2024 · 6 comments · Fixed by #17331
Closed

The argocd server api-content-type flag does not allow empty content-type header #17057

Nibiii opened this issue Jan 31, 2024 · 6 comments · Fixed by #17331
Labels
bug Something isn't working component:api API bugs and enhancements regression Bug is a regression, should be handled with high priority

Comments

@Nibiii
Copy link

Nibiii commented Jan 31, 2024

Summary

After recent security patch there is no way to accept api requests without content-type header set. The comment here says it should be possible, but it is not. As you can see, the default flag value is either environment variable or "application/json". Setting the default value to env.StringFromEnv("ARGOCD_API_CONTENT_TYPES", "") would allow incoming requests without content-type set to be processed.

@Nibiii Nibiii added the enhancement New feature or request label Jan 31, 2024
@Nibiii Nibiii changed the title The argocd server api-content-type flag does not allow empty content-type The argocd server api-content-type flag does not allow empty content-type header Jan 31, 2024
@crenshaw-dev
Copy link
Collaborator

What version are you running?

@Nibiii
Copy link
Author

Nibiii commented Feb 8, 2024

The problem affects all of the releases that include the fix for security issue GHSA-92mw-q256-5vwg, so that would be 2.7.16, 2.7.17, 2.8.8, 2.8.9, 2.8.10, 2.9.4, 2.9.5, 2.9.6 and 2.9.10. As i stated before, this pull request does not provide the functionality as described.

@Goorsky1
Copy link

It seems that this pull request attempted to allow disabling enforcing content type. However the issue is still relevant. Due to using StringFromEnv func (see func implementation) for setting the default flag value, when the user sets server.api.content.types to "", the default value application/json is still used. It's still impossible to set an empty value.

@crenshaw-dev
Copy link
Collaborator

@Goorsky1 hm, I tested that exact case and am pretty certain that setting an empty string in argocd-cmd-params-cm works. Can you share your test steps?

@jessesuen
Copy link
Member

@crenshaw-dev we have a user that has also hit this. The flaw is here:

https://github.com/argoproj/argo-cd/blob/v2.8.10/util/env/env.go#L127-L132

func StringFromEnv(env string, defaultValue string) string {
	if str := os.Getenv(env); str != "" {    // if env is "", it will return defaultValue, which is application/json
		return str
	}
	return defaultValue
}

I think the fix will need to delineate if the env var is not set vs. the env var is set to empty string and disable content-type enforcement if the latter.

@jessesuen jessesuen added bug Something isn't working regression Bug is a regression, should be handled with high priority component:api API bugs and enhancements and removed enhancement New feature or request labels Feb 27, 2024
@jessesuen
Copy link
Member

the fix will need to delineate if the env var is not set vs. the env var is set to empty string

In other words, os.Getenv doesn't tell us enough information. We would need to do this instead:

val, present := os.LookupEnv("ARGOCD_API_CONTENT_TYPES")

and disable content-type validation if present is true and val is ""

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component:api API bugs and enhancements regression Bug is a regression, should be handled with high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants