Conversation
Signed-off-by: Kevin Cui <bh@bugs.cc>
Summary by CodeRabbit
WalkthroughThis PR adds optional STS token support to the client library. Changes include: adding an optional Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Areas to focus on:
Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for Alibaba Cloud Security Token Service (STS) tokens to enable temporary credentials authentication in the TableStore SDK.
- Added optional
stsTokenfield to theClientConfiginterface - Implemented conditional STS token header injection in the request flow
- Updated documentation to demonstrate STS token usage
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/type.ts | Added optional stsToken field to ClientConfig interface |
| src/request.ts | Added import for H_OTS_STS_TOKEN constant and conditional logic to include STS token in request headers |
| src/const.ts | Defined H_OTS_STS_TOKEN constant for the STS token header name |
| cspell.config.yml | Added "ststoken" to spell-check dictionary |
| README.md | Updated usage example to show optional STS token configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (1)
24-24: Document token refresh and env usage (optional).Consider adding a brief note that STS tokens expire and showing
TABLESTORE_SESSION_TOKENenv-based initialization to guide users on rotation. This mirrors Alibaba docs and avoids hardcoding secrets. (alibabacloud.com)src/request.ts (1)
31-31: Prefer HTTPS by default (optional, out of scope).To avoid sending credentials over plain HTTP when
endpointlacks a scheme, default tohttps://if none is present.- const url = `http://${this.config.endpoint}/${options.apiName}`; + const hasScheme = /^https?:\/\//i.test(this.config.endpoint); + const base = hasScheme ? this.config.endpoint : `https://${this.config.endpoint}`; + const url = `${base}/${options.apiName}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)cspell.config.yml(1 hunks)src/const.ts(1 hunks)src/request.ts(2 hunks)src/type.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/request.ts (1)
src/const.ts (1)
H_OTS_STS_TOKEN(12-12)
🔇 Additional comments (5)
cspell.config.yml (1)
12-12: LGTM — dictionary updated appropriately.src/request.ts (2)
24-26: Good: inject STS header before signing.Ordering is correct, so the token participates in the signature. Consider adding a small test to assert the canonicalized headers include
x-ots-ststokenwhen provided.
4-4: Import looks correct and scoped.src/type.ts (1)
7-7: LGTM — optional stsToken matches ecosystem conventions.Property name aligns with examples in Alibaba docs and community usage. (alibabacloud.com)
src/const.ts (1)
12-12: I'll search for the official Tablestore documentation and Node.js SDK to verify the exact HTTP header name.Let me search more specifically for the HTTP header format in the official SDK implementation.
Let me search more specifically for Tablestore request headers and signing protocol.
Verify
x-ots-ststokenHTTP header naming against Node.js SDK implementation.Based on available public documentation, I was unable to find explicit confirmation of the exact HTTP header name that Tablestore expects for STS tokens. While the official Tablestore docs and SDKs confirm STS token support exists, they do not publicly expose the underlying HTTP header specification—Tablestore SDKs abstract this detail.
The constant name
x-ots-ststokenfollows the typicalx-ots-prefix pattern for Tablestore headers and appears reasonable, but without access to the official Node.js SDK source code or Tablestore's HTTP protocol specification, I cannot definitively verify its correctness. A mismatch here would indeed break authentication/signing as noted.
No description provided.