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

Pipeline Artifact API provides upload action with external app URL #32554

Open
jannispl opened this issue Nov 18, 2024 · 14 comments · May be fixed by #32564
Open

Pipeline Artifact API provides upload action with external app URL #32554

jannispl opened this issue Nov 18, 2024 · 14 comments · May be fixed by #32564
Labels

Comments

@jannispl
Copy link

Description

My setup is powered by Docker Compose, and I have Gitea and a Gitea Runner instance in the same Docker network. New task containers are also in this network, this is done by setting container.network: "gitea_default" in the runner configuration file.
There is an external reverse proxy pointing at Gitea for requests from outside. Internal requests are supposed to fail because of a IP allowlist - and I cannot allow the internal Gitea network. Thus, I have to rely on LOCAL_ROOT_URL to be used for all internal communication.

Unfortunately, it seems that when using the actions/upload-artifact@v3 action, the runner tries to access Gitea using the external ROOT_URL: (this is with ACTIONS_STEP_DEBUG=true)

followSymbolicLinks '***'
implicitDescendants '***'
omitBrokenSymbolicLinks '***'
excludeHiddenFiles '***'
followSymbolicLinks '***'
implicitDescendants '***'
matchDirectories '***'
omitBrokenSymbolicLinks '***'
excludeHiddenFiles '***'
Search path '/workspace/user/repo/test.txt'
File:/workspace/user/repo/test.txt was found using the provided searchPath
With the provided path, there will be 1 file uploaded
Root artifact directory is /workspace/user/repo
Starting artifact upload
For more detailed logs during the artifact upload process, enable step-debugging: https://docs.github.com/actions/monitoring-and-troubleshooting-workflows/enabling-debug-logging#enabling-step-debug-logging
Artifact name is valid!
Artifact Url: http://gitea:3000/api/actions_pipeline/_apis/pipelines/workflows/12/artifacts?api-version=6.0-preview
Upload Resource URL: https://git.example.org/api/actions_pipeline/_apis/pipelines/workflows/12/artifacts/dd18bf3a8e0a2a3e53e2661c7fb53534/upload
Container for artifact "test.txt" successfully created. Starting upload of file(s)
File Concurrency: 2, and Chunk Size: 8388608
/workspace/user/repo/test.txt is less than 64k in size. Creating a gzip file in-memory to potentially reduce the upload size
The gzip file created for /workspace/user/repo/test.txt did not help with reducing the size of the file. The original file will be uploaded as-is
::error::Unexpected response. Unable to upload chunk to https://git.example.org/api/actions_pipeline/_apis/pipelines/workflows/12/artifacts/dd18bf3a8e0a2a3e53e2661c7fb53534/upload?itemPath=test.txt%252Ftest.txt
##### Begin Diagnostic HTTP information #####
Status Code: 403
Status Message: Forbidden
Header Information: {
  "date": "Mon, 18 Nov 2024 21:34:19 GMT",
  "content-length": "9"
}
###### End Diagnostic HTTP information ######
::warning::Aborting upload for /workspace/user/repo/test.txt due to failure
::error::aborting artifact upload
Total size of all the files uploaded is 0 bytes
File upload process has finished. Finalizing the artifact upload
Artifact Url: http://gitea:3000/api/actions_pipeline/_apis/pipelines/workflows/12/artifacts?api-version=6.0-preview
URL is http://gitea:3000/api/actions_pipeline/_apis/pipelines/workflows/12/artifacts?api-version=6.0-preview&artifactName=test.txt
Finalize artifact upload - Attempt 1 of 5 failed with error: Artifact service responded with 500
...

As you can see, Upload Resource URL is resolved based on ROOT_URL rather than LOCAL_ROOT_URL. A similar error appears with (the patched variant of) v4.

I took a look at the source code for actions/upload-artifact@v3 and stumbled upon this: https://github.com/actions/toolkit/blob/%40actions/artifact%401.1.1/packages/artifact/src/internal/artifact-client.ts#L118

Here, it appears the URL is provided by an API endpoint. In Gitea, the fileContainerResourceUrl is computed here: https://github.com/go-gitea/gitea/blob/main/modules/httplib/url.go#L61

It seems to account for the Host header here, but only if the request is coming from a reverse proxy (X-Forwarded-Proto present).
I realized I can workaround this problem by running an additional reverse proxy for internal requests, but I'd rather avoid that.

Gitea Version

1.22.3

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

docker compose

How are you running Gitea?

I am using official Gitea Docker images on the latest tag.

Database

SQLite

@wxiaoguang
Copy link
Contributor

LOCAL_ROOT_URL is only designed to be used by gitea's builtin commands, it is never expected to be used out of the local network.

In all cases, you should use ROOT_URL

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 19, 2024

It seems to account for the Host header here, but only if the request is coming from a reverse proxy (X-Forwarded-Proto present).

If you are running Gitea behind a proxy, then you must pass X-Forwarded-Proto correctly, then all URLs should be correct, then you don't need to touch the LOCAL_ROOT_URL

https://docs.gitea.com/administration/reverse-proxies#general-configuration

@jannispl
Copy link
Author

I am running Gitea behind a proxy for external requests. I do not expect internal requests coming from runner containers passing through the reverse proxy though, this is why I set LOCAL_ROOT_URL to the container hostname, bypassing the reverse proxy.

@wxiaoguang
Copy link
Contributor

I am running Gitea behind a proxy for external requests. I do not expect internal requests coming from runner containers passing through the reverse proxy though, this is why I set LOCAL_ROOT_URL to the container hostname, bypassing the reverse proxy.

But LOCAL_ROOT_URL was never designed to be used for non-gitea requests. It is totally an internal API endpoint and will change without any compatibility guarantee.

I do not see why "not expect internal requests coming from runner containers passing through the reverse proxy though", they are not "internal", it is a right approach and should be no security/performance problem.

@wxiaoguang
Copy link
Contributor

Internal requests are supposed to fail because of a IP allowlist - and I cannot allow the internal Gitea network.

This is the point that I do not understand, internal traffic are generally safer then external, but internals are denied?

@jannispl
Copy link
Author

jannispl commented Nov 19, 2024

This is the point that I do not understand, internal traffic are generally safer then external, but internals are denied?

The problem is that the Docker network is dynamic enough that I don't have a specific IP address to whitelist.

Since Gitea supports running without a reverse proxy, I expected to be able to route requests from runners to the instance directly.

But LOCAL_ROOT_URL was never designed to be used for non-gitea requests. It is totally an internal API endpoint and will change without any compatibility guarantee.

Could you clarify why LOCAL_ROOT_URL is still being used for the first "Artifact URL" mentioned in my runner logs?

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 19, 2024

Since Gitea supports running without a reverse proxy, I expected to be able to route requests from runners to the instance directly.

Maybe there could be some approaches like this, make the containers could use docker host's IP as gitea server's IP , then accessing "http://gitea-server:3000" will be able to go to the "gitea" container.

services:
  gitea:
      set ROOT_URL to "gitea-server"
     ports: 3000:3000
  action-runner:
    extra_hosts:
      - "gitea-server:host-gateway"

Could you clarify why LOCAL_ROOT_URL is still being used for the first "Artifact URL" mentioned in my runner logs?

I just read the description, it said:

As you can see, Upload Resource URL is resolved based on ROOT_URL rather than LOCAL_ROOT_URL

So I think it just uses "ROOT_URL" as expected?

If you think there is a problem, please show your full config details.

@jannispl
Copy link
Author

Maybe there could be some approaches like this, make the containers could use docker host's IP as gitea server's IP , then accessing "http://gitea-server:3000/" will be able to go to the "gitea" container.

Not a feasible solution in my case, as I do not want to publish any ports on the host interface.


There are two mentions of URLs in the runner log: first one is resolved using LOCAL_ROOT_URL, second one using ROOT_URL:

...
Artifact name is valid!
Artifact Url: http://gitea:3000/api/actions_pipeline/_apis/pipelines/workflows/12/artifacts?api-version=6.0-preview
Upload Resource URL: https://git.example.org/api/actions_pipeline/_apis/pipelines/workflows/12/artifacts/dd18bf3a8e0a2a3e53e2661c7fb53534/upload
...

@jannispl
Copy link
Author

My compose file looks like this:

services:
  gitea:
    container_name: gitea
    image: gitea/gitea:latest
    restart: unless-stopped
    environment:
      - USER_UID=1000
      - USER_GID=1000
    networks:
      - proxy
      - default
    volumes:
      - "data:/data"
      - "/etc/timezone:/etc/timezone:ro"
      - "/etc/localtime:/etc/localtime:ro"

  runner:
    container_name: gitea-runner
    image: gitea/act_runner:latest
    restart: unless-stopped
    environment:
      CONFIG_FILE: /config/config.yaml
      GITEA_INSTANCE_URL: "http://gitea:3000"
      GITEA_RUNNER_REGISTRATION_TOKEN: "xxx"
      GITEA_RUNNER_NAME: "gitea-runner"
      GITEA_RUNNER_LABELS: ""
    volumes:
      - runner-config:/config
      - runner-data:/data
      - /var/run/docker.sock:/var/run/docker.sock

volumes:
  data: {}
  runner-config: {}
  runner-data: {}

networks:
  proxy:
    external: true
    name: proxy

relevant server section in app.ini:

[server]
APP_DATA_PATH = /data/gitea
DOMAIN = git.example.org
HTTP_PORT = 3000
ROOT_URL = https://git.example.org/
LOCAL_ROOT_URL = http://gitea:3000/
DISABLE_SSH = true
LFS_START_SERVER = false
OFFLINE_MODE = false

runner config:

container:
  # Specifies the network to which the container will connect.
  # Could be host, bridge or the name of a custom network.
  # If it's empty, act_runner will create a network automatically.
  network: "gitea_default"

@jannispl
Copy link
Author

Maybe there could be some approaches like this, make the containers could use docker host's IP as gitea server's IP , then accessing "http://gitea-server:3000/" will be able to go to the "gitea" container.

That would also not work if ROOT_URL is using https

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 19, 2024

There are two mentions of URLs in the runner log: first one is resolved using LOCAL_ROOT_URL, second one using ROOT_URL:

It seems that it is from GITEA_INSTANCE_URL: "http://gitea:3000". Gitea never uses "LOCAL_ROOT_URL" for non-gitea builtin purpose (builtin means only some gitea sub-commands internally)


If you would really like make the runner use container's network, it could be like this (still, not perfect, but maybe it works for your usage)

  1. Set X-Forwarded-Proto correctly, then end users could use the web interface correctly
  2. Set ROOT_URL to http://gitea:3000 and revert LOCAL_ROOT_URL

Otherwise, I would suggest you to setup a new nginx container in the docker compose to handle "runner" requests, but not using LOCAL_ROOT_URL (please forget it ....)

@jannispl
Copy link
Author

jannispl commented Nov 19, 2024

It seems that it is from GITEA_INSTANCE_URL: "http://gitea:3000".

I see, it looks like I was misled there.

Set X-Forwarded-Proto correctly, then end users could use the web interface correctly

On the existing reverse proxy I am already doing this.

Set ROOT_URL to http://gitea:3000 and revert LOCAL_ROOT_UR

I have actually tried this and it resulted in a big warning banner telling me to fix my ROOT_URL.

I guess I will investigate the routing from runner to reverse proxy, make it somewhat deterministic and see if I can find a way to allow traffic.

Otherwise, I would suggest you to setup a new nginx container in the docker compose to handle "runner" requests

Possible but not great because of two servers fighting for SSL

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 19, 2024

There could be another choice, you could apply a simple patch to build your own program, or introduce a new option to control the "guessing" behavior:

	if reqScheme == "" {
		return "http://" + req.Host
	}

Then:

  • Set X-Forwarded-Proto correctly (it is always a must)
  • When your runner accesses "http://gitea:3000", there is no "reqScheme", then it uses the host in the request URL http://gitea:3000.

This approach wasn't used to "guess" the host due to the comment then case 2 would result in wrong guess like guessed AppURL becomes "http://gitea:3000/", which is not accessible by end users. , because it shouldn't break existing users (because many users do not follow the document to setup X-Forwarded-Proto header correctly, they just wrote the reverse proxy config by intuition)

(well, the comment also said so: So in the future maybe it should introduce a new config option, to let site admin decide how to guess the AppURL.)

jannispl added a commit to jannispl/gitea that referenced this issue Nov 19, 2024
Resolves go-gitea#32554

This option can be set to make Gitea always use the "Host" request header for construction of absolute URLs.
jannispl added a commit to jannispl/gitea that referenced this issue Nov 19, 2024
Resolves go-gitea#32554

This option can be set to make Gitea always use the "Host" request header for construction of absolute URLs.
jannispl added a commit to jannispl/gitea that referenced this issue Nov 19, 2024
Resolves go-gitea#32554

This option can be set to make Gitea always use the "Host" request header for construction of absolute URLs.
@jannispl jannispl linked a pull request Nov 19, 2024 that will close this issue
jannispl added a commit to jannispl/gitea that referenced this issue Nov 19, 2024
Resolves go-gitea#32554

This option can be set to make Gitea always use the "Host" request header for construction of absolute URLs.
@jannispl
Copy link
Author

jannispl commented Nov 19, 2024

For now, I have solved this issue by configuring the runner to launch containers in a network that my reverse proxy is also part of. This, in combination with an extra argument --add-host git.example.org:172.xx.xx.xx allows me to point runner tasks to the reverse proxy using its internal address. Now I have a subnet that I can properly allow access for in the reverse proxy. The downside to this is that I can only attach runner containers to a single network, but I can live with that. Also, having to use a static IP for the reverse proxy bugs me a little.

Nevertheless I have opened a PR #32564 that adds a new config option which would allow targeting Gitea without a reverse proxy - provided that any potential proxy is configured to pass the correct headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants