Skip to content

Conversation

@xwid138
Copy link

@xwid138 xwid138 commented Jan 8, 2026

When reading a path not starting with /pub/ Homeserver responds with Writing to directories other than '/pub/' is forbidden error message which is confusing for GET requests.

WARN request{method=GET uri="/test/hello.txt" version=HTTP/1.1}: pubky_homeserver::client_server::layers::authz: Writing to directories other than '/pub/' is forbidden: 8kcqwm7fw43j73jo4s4thzawzhjodc6zrn9cjyko4pzwxpkmwwey//test/hello.txt. Access forbidden
cargo run user get /test/hello.txt ./recovery.key
.
.
.
Error: Failed to get data

Caused by:
    0: Request failed: Server responded with an error: 403 Forbidden - Writing to directories other than '/pub/' is forbidden
    1: Server responded with an error: 403 Forbidden - Writing to directories other than '/pub/' is forbidden

After change:

Error: Failed to get data

Caused by:
    0: Request failed: Server responded with an error: 400 Bad Request - Invalid URL: Path must start with /pub/
    1: Server responded with an error: 400 Bad Request - Invalid URL: Path must start with /pub/

Copy link
Contributor

@SHAcollision SHAcollision left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey xwid138:fix-authz thank you for bringing this up and coming up with a fix. 🙌

I left a comment for a different take of this fix that could equally solve the problem you exposed without opening up some possible unsafe behaviour.

// return Ok(());
// }
} else {
} else if method == Method::PUT {
Copy link
Contributor

@SHAcollision SHAcollision Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could potentially allow other writing methods like DELETE into non /pub/ paths.

Feels like this whole conditional is hacky, but it's kindda correct "as is". So we might simply want to change L123 and L128 messages to "Access to non-/pub/ paths is forbidden” so it is accurate for reads too. What do you think?

Copy link
Author

@xwid138 xwid138 Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true - my bad.

I suggest adding some unit tests to authz, just to be safe. Wdyt ?

edit: should we allow for OPTIONS method as well in case of /pub/ path ? Only GET and HEADER are allowed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Message update.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants