Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 43 additions & 42 deletions crates/aisix-guardrail/src/guardrails/bedrock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,10 @@ const PATH_SEGMENT_ENCODE_SET: &AsciiSet = &CONTROLS
.add(b'}')
.add(b'/');

#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
#[derive(Clone, Serialize, Deserialize, ToSchema)]
pub struct BedrockGuardrailConfig {
pub identifier: String,
pub version: String,
}

#[derive(Clone, Serialize, Deserialize, ToSchema)]
pub struct BedrockRuntimeConfig {
pub region: String,
pub access_key_id: String,
pub secret_access_key: String,
Expand All @@ -58,11 +54,13 @@ pub struct BedrockRuntimeConfig {
pub endpoint: Option<String>,
}

impl fmt::Debug for BedrockRuntimeConfig {
impl fmt::Debug for BedrockGuardrailConfig {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
const REDACTED: &str = "[REDACTED]";

f.debug_struct("BedrockRuntimeConfig")
f.debug_struct("BedrockGuardrailConfig")
.field("identifier", &self.identifier)
.field("version", &self.version)
.field("region", &self.region)
.field("access_key_id", &REDACTED)
.field("secret_access_key", &REDACTED)
Expand All @@ -87,37 +85,40 @@ impl GuardrailMeta for BedrockGuardrailMeta {
#[derive(Debug, Clone)]
pub struct BedrockGuardrailRuntime {
client: Client,
runtime: BedrockRuntimeConfig,
}

impl Default for BedrockGuardrailRuntime {
fn default() -> Self {
Self::new()
}
}

impl BedrockGuardrailRuntime {
pub fn new(runtime: BedrockRuntimeConfig) -> Self {
pub fn new() -> Self {
Self {
client: Client::new(),
runtime,
}
}

pub fn with_client(client: Client, runtime: BedrockRuntimeConfig) -> Self {
Self { client, runtime }
pub fn with_client(client: Client) -> Self {
Self { client }
}

fn build_apply_url(&self, guardrail: &BedrockGuardrailConfig) -> Result<Url, BedrockError> {
let base_url = self
.runtime
fn build_apply_url(&self, config: &BedrockGuardrailConfig) -> Result<Url, BedrockError> {
let base_url = config
.endpoint
.as_deref()
.map(str::to_owned)
.unwrap_or_else(|| {
format!(
"{DEFAULT_RUNTIME_HOST_PREFIX}{}{DEFAULT_RUNTIME_HOST_SUFFIX}",
self.runtime.region
config.region
)
});
let endpoint_path = format!(
"/guardrail/{}/version/{}/apply",
encode_path_segment(&guardrail.identifier),
encode_path_segment(&guardrail.version),
encode_path_segment(&config.identifier),
encode_path_segment(&config.version),
);

build_url_for_endpoint(&base_url, &endpoint_path)
Expand Down Expand Up @@ -149,6 +150,7 @@ impl BedrockGuardrailRuntime {
url: &Url,
body: &[u8],
time: SystemTime,
config: &BedrockGuardrailConfig,
) -> Result<SignedRequest, BedrockError> {
let mut headers = HeaderMap::new();
headers.insert(CONTENT_TYPE, HeaderValue::from_static("application/json"));
Expand All @@ -167,16 +169,16 @@ impl BedrockGuardrailRuntime {
.collect::<Result<Vec<_>, BedrockError>>()?;

let identity: Identity = Credentials::new(
self.runtime.access_key_id.clone(),
self.runtime.secret_access_key.clone(),
self.runtime.session_token.clone(),
config.access_key_id.clone(),
config.secret_access_key.clone(),
config.session_token.clone(),
None,
"aisix-bedrock-guardrail-static",
)
.into();
let signing_params = v4::SigningParams::builder()
.identity(&identity)
.region(self.runtime.region.as_str())
.region(config.region.as_str())
.name("bedrock")
.time(time)
.settings(SigningSettings::default())
Expand Down Expand Up @@ -234,7 +236,7 @@ impl GuardrailRuntime<BedrockGuardrailConfig> for BedrockGuardrailRuntime {
};

let url = self.build_apply_url(config)?;
let signed = self.sign_request(&url, &body, SystemTime::now())?;
let signed = self.sign_request(&url, &body, SystemTime::now(), config)?;
let response = self
.client
.post(signed.url)
Expand Down Expand Up @@ -430,22 +432,27 @@ mod tests {

use super::{
ApplyGuardrailOutput, ApplyGuardrailResponse, BedrockError, BedrockGuardrailConfig,
BedrockGuardrailRuntime, BedrockRuntimeConfig, build_url_for_endpoint,
outcome_from_response,
BedrockGuardrailRuntime, build_url_for_endpoint, outcome_from_response,
};
use crate::traits::{
GuardrailCheckPayload, GuardrailMessage, GuardrailMessageContent, GuardrailOutcome,
GuardrailRole, InputGuardrailPayload,
};

fn runtime() -> BedrockGuardrailRuntime {
BedrockGuardrailRuntime::new(BedrockRuntimeConfig {
fn config() -> BedrockGuardrailConfig {
BedrockGuardrailConfig {
identifier: "guardrail-123".into(),
version: "1".into(),
region: "us-east-1".into(),
access_key_id: "AKIA123".into(),
secret_access_key: "secret".into(),
session_token: Some("token".into()),
endpoint: Some("https://bedrock-runtime.us-east-1.amazonaws.com/guardrail".into()),
})
}
}

fn runtime() -> BedrockGuardrailRuntime {
BedrockGuardrailRuntime::new()
}

fn input_payload(text: &str) -> GuardrailCheckPayload {
Expand All @@ -461,14 +468,8 @@ mod tests {
}

#[test]
fn bedrock_runtime_config_debug_redacts_credentials() {
let config = BedrockRuntimeConfig {
region: "us-east-1".into(),
access_key_id: "AKIA123".into(),
secret_access_key: "secret".into(),
session_token: Some("token".into()),
endpoint: Some("https://bedrock-runtime.us-east-1.amazonaws.com".into()),
};
fn bedrock_guardrail_config_debug_redacts_credentials() {
let config = config();

let output = format!("{config:?}");
assert!(output.contains("[REDACTED]"));
Expand All @@ -494,12 +495,10 @@ mod tests {
#[test]
fn build_apply_url_percent_encodes_identifier_segments() {
let runtime = runtime();
let url = runtime
.build_apply_url(&BedrockGuardrailConfig {
identifier: "guardrail/name".into(),
version: "DRAFT".into(),
})
.unwrap();
let mut config = config();
config.identifier = "guardrail/name".into();
config.version = "DRAFT".into();
let url = runtime.build_apply_url(&config).unwrap();

assert!(
url.path()
Expand All @@ -510,6 +509,7 @@ mod tests {
#[test]
fn sign_request_adds_sigv4_authorization_header() {
let runtime = runtime();
let config = config();
let signed = runtime
.sign_request(
&Url::parse(
Expand All @@ -518,6 +518,7 @@ mod tests {
.unwrap(),
br#"{"content":[{"text":{"text":"hello"}}],"source":"INPUT"}"#,
UNIX_EPOCH + Duration::from_secs(1_700_000_000),
&config,
)
.unwrap();

Expand Down
2 changes: 1 addition & 1 deletion crates/aisix-guardrail/src/guardrails/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
pub mod bedrock;

pub use bedrock::{BedrockGuardrailMeta, BedrockGuardrailRuntime, BedrockRuntimeConfig};
pub use bedrock::{BedrockGuardrailMeta, BedrockGuardrailRuntime};

pub mod identifiers {
use super::bedrock;
Expand Down
49 changes: 49 additions & 0 deletions src/config/entities/guardrails-schema.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
{
"$schema": "https://json-schema.org/draft/2020-12/schema#",
"type": "object",
"properties": {
"name": { "type": "string", "minLength": 1 },
"type": {
"type": "string",
"enum": ["bedrock"]
},
"config": { "type": "object" }
},
"required": ["name", "type", "config"],
"additionalProperties": false,
"allOf": [
{
"if": {
"properties": { "type": { "const": "bedrock" } },
"required": ["type"]
},
"then": {
"properties": {
"config": { "$ref": "#/$defs/bedrock" }
}
}
}
],
"$defs": {
"bedrock": {
"type": "object",
"required": [
"identifier",
"version",
"region",
"access_key_id",
"secret_access_key"
],
"properties": {
"identifier": { "type": "string", "minLength": 1 },
"version": { "type": "string", "minLength": 1 },
"region": { "type": "string", "minLength": 1 },
"access_key_id": { "type": "string", "minLength": 1 },
"secret_access_key": { "type": "string", "minLength": 1 },
Comment on lines +34 to +42
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Security concern: Credentials stored in plain text without secret manager support.

The schema requires access_key_id, secret_access_key, and optional session_token as plain strings, which means AWS credentials will be stored in clear text in the configuration backend (etcd). This creates significant security risks:

  • Credentials are exposed in configuration snapshots, logs, and backups
  • No rotation mechanism without config updates
  • Violates least-privilege principle for config access

Consider supporting secret manager references (e.g., $secret://path/to/credential) in these fields so credentials can be resolved at runtime from a secure vault. As per coding guidelines, environment variable and secret manager references in configuration fields must be properly resolved before use, implying the schema should allow such references.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/config/entities/guardrails-schema.json` around lines 34 - 42, The JSON
schema currently requires plaintext "access_key_id" and "secret_access_key" (and
similarly "session_token" if used); change the schema for these properties so
they can accept secret-manager references (e.g., a string like "$secret://...")
or an explicit secret reference object rather than only a raw credential.
Concretely, update the "properties" entries for "access_key_id",
"secret_access_key" (and "session_token" if present) to allow either a non-empty
string OR a secret reference (e.g., a string matching a "^\\$secret://.+$"
pattern or a union with an object form like {"secret_ref":"<string>"}), and
adjust any "minLength" constraints accordingly; ensure the schema still lists
these names in the required array only if truly required, and add schema
documentation so runtime code that reads these fields (credential resolution
logic) can detect and resolve "$secret://" references before use.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Link to #58

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

gh issue view 58 --repo api7/aisix

Length of output: 1384


@bzp2010, thanks for the reference! The security concern raised here — storing AWS credentials as plain strings in the config backend — is already tracked in issue #58 ("feat: support secret references and secret provider integrations for provider configs"), which proposes support for $env:// and $secret:// reference syntax for runtime credential resolution. I'll note this as a known, tracked limitation for now.


✏️ Learnings added
Learnt from: bzp2010
Repo: api7/aisix PR: 103
File: src/config/entities/guardrails-schema.json:34-42
Timestamp: 2026-05-12T09:15:42.333Z
Learning: In the api7/aisix repository, plaintext credential storage for sensitive fields (e.g., `access_key_id`, `secret_access_key`, `api_key`) in provider/guardrail config schemas is a known, accepted limitation tracked in GitHub issue `#58`. The planned solution is to support `$env://` and `$secret://` reference syntax for runtime credential resolution via secret provider integrations. Do not raise this as a blocking concern in reviews; instead, reference issue `#58`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

"session_token": { "type": "string", "minLength": 1 },
"endpoint": { "type": "string", "minLength": 1 }
},
"additionalProperties": false
}
}
}
Loading