Skip to content

feat(storage): add signed post policy v4#5576

Open
dmitri-lerko wants to merge 2 commits intogoogleapis:mainfrom
dmitri-lerko:feat/storage-go-parity
Open

feat(storage): add signed post policy v4#5576
dmitri-lerko wants to merge 2 commits intogoogleapis:mainfrom
dmitri-lerko:feat/storage-go-parity

Conversation

@dmitri-lerko
Copy link
Copy Markdown

@dmitri-lerko dmitri-lerko commented May 3, 2026

Summary

  • add canonical Rust Signed V4 POST Policy support for Cloud Storage using the existing signer abstraction
  • expose PostPolicyV4Builder, PostPolicyV4, PolicyV4Fields, and policy condition helpers from google-cloud-storage
  • validate local protocol invariants required to construct a valid policy, including bucket name shape, expiration bounds, conflicting exact-match fields, and unsafe metadata fields
  • keep this PR scoped to POST Policy only per maintainer feedback; no src/gax, src/gax-internal, Requester Pays, emulator, or report/plan Markdown changes are included

Testing

  • cargo test -p google-cloud-storage post_policy
  • cargo clippy -p google-cloud-storage --all-targets -- --deny warnings
  • git diff --check

Latest local focused result: 19 passed, 623 filtered out for cargo test -p google-cloud-storage post_policy.

@google-cla
Copy link
Copy Markdown

google-cla Bot commented May 3, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label May 3, 2026
@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces support for Signed V4 POST policies in the Rust Cloud Storage SDK, aligning it with the capabilities of the Go Storage package. It also establishes a formal roadmap and testing framework to ensure future parity efforts are consistent, well-documented, and rigorously validated against Go reference behaviors.

Highlights

  • Signed V4 POST Policy Support: Added Rust implementation for Signed V4 POST policies, enabling unauthenticated uploads via multipart HTML forms with robust condition validation.
  • Go/Rust Parity Plan: Introduced a comprehensive parity plan, including a capability matrix and a durable report to track and guide future efforts toward feature parity with the Go Storage SDK.
  • Conformance and Boundary Testing: Implemented extensive test coverage, including Go-equivalent conformance fixtures, boundary tests for metadata and expiration, and property-based checks for JSON escaping.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request establishes a comprehensive plan and initial implementation for bringing the Rust Storage SDK toward parity with the Go Storage library. It introduces a detailed parity report and matrix to track progress across various feature groups, such as ACLs, HMAC keys, and notifications. The initial implementation slice adds support for Signed V4 POST policies, including a new builder, condition types, and extensive conformance testing against existing cross-language fixtures. The feedback provided suggests a minor documentation refinement to point directly to the implementation file in the parity matrix for better clarity.

Comment thread docs/reports/storage-go-parity.md Outdated
| HMAC keys | `storage.HMACKeyHandle` and client methods | Missing | No Rust HMAC helper module | No Rust parity tests yet |
| Pub/Sub notifications | `storage.Notification` helpers | Missing | No Rust notification helper module | No Rust parity tests yet |
| Signed V4 URLs | `storage.SignedURL` and bucket signed URLs | Implemented | `src/storage/src/storage/signed_url.rs` | Cross-language conformance coverage exists |
| Signed V4 POST policies | `storage.GenerateSignedPostPolicyV4` | Implemented | `src/storage/src/storage/post_policy_v4.rs`, `src/storage/src/post_policy_v4.rs` | Go-equivalent conformance, boundary, endpoint style, and Send tests exist |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For clarity, you could remove the redundant path to the re-export module from the Evidence column, so it points directly to the implementation file.

Suggested change
| Signed V4 POST policies | `storage.GenerateSignedPostPolicyV4` | Implemented | `src/storage/src/storage/post_policy_v4.rs`, `src/storage/src/post_policy_v4.rs` | Go-equivalent conformance, boundary, endpoint style, and Send tests exist |
| Signed V4 POST policies | `storage.GenerateSignedPostPolicyV4` | Implemented | `src/storage/src/storage/post_policy_v4.rs` | Go-equivalent conformance, boundary, endpoint style, and Send tests exist |

@dmitri-lerko dmitri-lerko changed the title feat(storage): add signed post policy v4 feat(storage): add signed post policy and requester pays parity May 3, 2026
@dmitri-lerko dmitri-lerko changed the title feat(storage): add signed post policy and requester pays parity feat(storage): add storage go parity features May 3, 2026
@coryan
Copy link
Copy Markdown
Contributor

coryan commented May 3, 2026

Thanks for the PR. You will need to sign the CLA or we cannot accept the PR at all.

Please split the changes to src/gax and src/gax-internal to a separate PR.

I think the work for user project duplicates the work in #5490

We would also prefer if you split the work for STORAGE_EMULATOR_HOST to a separate PR from the work for the POST Policy changes.

@joshuatants I believe most of the review here will fall on your shoulders.

@dmitri-lerko dmitri-lerko force-pushed the feat/storage-go-parity branch from 405dbd0 to 89e396f Compare May 3, 2026 20:04
@dmitri-lerko dmitri-lerko force-pushed the feat/storage-go-parity branch from 89e396f to dc5b7bb Compare May 3, 2026 20:08
@dmitri-lerko dmitri-lerko changed the title feat(storage): add storage go parity features feat(storage): add signed post policy v4 May 3, 2026
@dmitri-lerko dmitri-lerko marked this pull request as ready for review May 3, 2026 20:18
@dmitri-lerko dmitri-lerko requested review from a team as code owners May 3, 2026 20:18
@dmitri-lerko
Copy link
Copy Markdown
Author

Thanks for the PR. You will need to sign the CLA or we cannot accept the PR at all.

Please split the changes to src/gax and src/gax-internal to a separate PR.

I think the work for user project duplicates the work in #5490

We would also prefer if you split the work for STORAGE_EMULATOR_HOST to a separate PR from the work for the POST Policy changes.

@joshuatants I believe most of the review here will fall on your shoulders.

Hi @coryan ,

Thank you for your prompt reply. I am happy to take it a step at a time. I'd love to see Go <> Rust SDK parity when it comes to Cloud Storage and Cloud Monitoring. For the latter I am running my own fork in production, but would love to move it over here piecemeal.

Please let me know what I can do to make your jobs as easy as possible here.

@joshuatants
Copy link
Copy Markdown
Contributor

Hi @dmitri-lerko thank you very much for this.

We actually have someone working on the signed post policy v4 and are tracking the issue here (#4746). I should've been more prompt with updating the issue to reflect this.

@xlai20 - can you have a look at Dmitri's work? How far along are you with your implementation? How much of Dmitri's work can be used?

@joshuatants joshuatants requested a review from xlai20 May 4, 2026 01:19
Copy link
Copy Markdown
Member

@xlai20 xlai20 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. It looks moving to the right direction. I've left some comments to request minor changes.


/// A constraint that the uploaded multipart form must satisfy.
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum PostPolicyV4Condition {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

According to https://docs.cloud.google.com/storage/docs/authentication/signatures#policy-document, there are 3 types of policy conditions: Exact Match, Starts With, and Content Length Range. As here it misses out the Exact Match type, user cannot have the freedom to define exact match condition on fields, whilst such capability is supported in other sdk.

Self::ContentLengthRange { start, end }
}

fn is_empty(&self) -> bool {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is_empty() function and its use in filtering out "invalid" policy document conditions is better to be removed in an SDK implementation.

As per the documentation https://docs.cloud.google.com/storage/docs/authentication/signatures#policy-document,

"If a field's value has no restrictions, use the starts-with condition with ["starts-with", "$field", ""],

this suggest that the GCS backend is expecting a starts-with empty string condition. Doing a filtering might end up passing incorrect inputs to the backend.

impl PostPolicyV4Condition {
/// Creates a `starts-with` condition.
///
/// Empty values are ignored, matching the Go Storage SDK behavior.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ditto my another comment, don't ignore empty values.

JsonCondition::Object(BTreeMap::from([(name.to_string(), value.into())]))
}

fn escape_like_go_json(json: &str) -> String {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rename this function to "escape_json_str" or "escape_invalid_char".

escaped
}

fn push_json_unicode_escape(output: &mut String, ch: char) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This function is a bit too low-level styled, unlike the usual high-level coding style in Rust, is there a way to improve it?

fn push_json_unicode_escape(output: &mut String, ch: char) {
let code = ch as u32;
if code <= 0xffff {
output.push_str(&format!("\\u{code:04x}"));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As this function is called within the loop of escape function, the use of format! can be a performance hit. Calling the format! macro always allocates a new String on the heap. Try to explore if can change to write! macro to avoid allocations.

@dmitri-lerko
Copy link
Copy Markdown
Author

Thanks for the PR. It looks moving to the right direction. I've left some comments to request minor changes.

Thank you for your guidance and thorough review, I'll aim to iterate on the feedback within a week,

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

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants