Skip to content

Commit af87abb

Browse files
nickpismenkovilblackdragon
authored andcommitted
fix: Google Sheets returns 403 PERMISSION_DENIED after completing OAuth (#1164)
* fix: Google Sheets returns 403 PERMISSION_DENIED after completing OAuth * fix: linter * fix: linter * fix: ci * fix * fix * fix * fix
1 parent e714df2 commit af87abb

27 files changed

Lines changed: 538 additions & 7 deletions

.github/workflows/e2e.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ jobs:
5252
- group: features
5353
files: "tests/e2e/scenarios/test_skills.py tests/e2e/scenarios/test_tool_approval.py"
5454
- group: extensions
55-
files: "tests/e2e/scenarios/test_extensions.py tests/e2e/scenarios/test_extension_oauth.py tests/e2e/scenarios/test_wasm_lifecycle.py tests/e2e/scenarios/test_tool_execution.py tests/e2e/scenarios/test_pairing.py"
55+
files: "tests/e2e/scenarios/test_extensions.py tests/e2e/scenarios/test_extension_oauth.py tests/e2e/scenarios/test_wasm_lifecycle.py tests/e2e/scenarios/test_tool_execution.py tests/e2e/scenarios/test_pairing.py tests/e2e/scenarios/test_oauth_credential_fallback.py tests/e2e/scenarios/test_routine_oauth_credential_injection.py"
5656
steps:
5757
- uses: actions/checkout@v6
5858

src/tools/wasm/wrapper.rs

Lines changed: 197 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,7 +1104,18 @@ async fn resolve_host_credentials(
11041104
) -> Vec<ResolvedHostCredential> {
11051105
let store = match store {
11061106
Some(s) => s,
1107-
None => return Vec::new(),
1107+
None => {
1108+
// If tool requires credentials but has no secrets store, this is a configuration error
1109+
if let Some(http_cap) = &capabilities.http
1110+
&& !http_cap.credentials.is_empty()
1111+
{
1112+
tracing::warn!(
1113+
user_id = %user_id,
1114+
"WASM tool requires credentials but secrets_store is not configured - authentication will fail"
1115+
);
1116+
}
1117+
return Vec::new();
1118+
}
11081119
};
11091120

11101121
// Check if the access token needs refreshing before resolving credentials.
@@ -1155,13 +1166,37 @@ async fn resolve_host_credentials(
11551166
continue;
11561167
}
11571168

1169+
// Try to get credential under the provided user_id first.
1170+
// If not found and user_id != "default", fallback to "default" (global credentials).
1171+
// This handles OAuth tokens stored globally under "default" but accessed from routine contexts.
11581172
let secret = match store.get_decrypted(user_id, &mapping.secret_name).await {
1159-
Ok(s) => s,
1173+
Ok(s) => Some(s),
11601174
Err(e) => {
1161-
tracing::debug!(
1175+
// If lookup fails and we're not already looking up "default", try "default" as fallback
1176+
if user_id != "default" {
1177+
tracing::debug!(
1178+
secret_name = %mapping.secret_name,
1179+
user_id = %user_id,
1180+
error = %e,
1181+
"Credential not found for user, trying default global credentials"
1182+
);
1183+
store
1184+
.get_decrypted("default", &mapping.secret_name)
1185+
.await
1186+
.ok()
1187+
} else {
1188+
None
1189+
}
1190+
}
1191+
};
1192+
1193+
let secret = match secret {
1194+
Some(s) => s,
1195+
None => {
1196+
tracing::warn!(
11621197
secret_name = %mapping.secret_name,
1163-
error = %e,
1164-
"Could not resolve credential for WASM tool (auth may not be configured)"
1198+
user_id = %user_id,
1199+
"Could not resolve credential for WASM tool (not found in user context or default)"
11651200
);
11661201
continue;
11671202
}
@@ -2058,4 +2093,161 @@ mod tests {
20582093
"Leak scan on post-injection headers should block the Slack token"
20592094
);
20602095
}
2096+
2097+
#[tokio::test]
2098+
async fn test_resolve_host_credentials_fallback_to_default_user() {
2099+
use crate::secrets::{CredentialLocation, CredentialMapping, SecretsStore};
2100+
use crate::tools::wasm::capabilities::HttpCapability;
2101+
use crate::tools::wasm::wrapper::resolve_host_credentials;
2102+
2103+
let store = test_secrets_store();
2104+
2105+
// Store a token under the "default" global user
2106+
store
2107+
.create(
2108+
"default",
2109+
crate::secrets::CreateSecretParams::new("google_oauth_token", "global_token_value"),
2110+
)
2111+
.await
2112+
.expect("Failed to store global token"); // safety: test code only
2113+
2114+
// Create capabilities requiring this credential
2115+
let mut creds = std::collections::HashMap::new();
2116+
creds.insert(
2117+
"google_oauth_token".to_string(),
2118+
CredentialMapping {
2119+
secret_name: "google_oauth_token".to_string(),
2120+
location: CredentialLocation::AuthorizationBearer,
2121+
host_patterns: vec!["sheets.googleapis.com".to_string()],
2122+
},
2123+
);
2124+
let caps = Capabilities {
2125+
http: Some(HttpCapability {
2126+
allowlist: vec![],
2127+
credentials: creds,
2128+
rate_limit: crate::tools::wasm::capabilities::RateLimitConfig::default(),
2129+
max_request_bytes: 1024 * 1024,
2130+
max_response_bytes: 10 * 1024 * 1024,
2131+
timeout: std::time::Duration::from_secs(30),
2132+
}),
2133+
..Default::default()
2134+
};
2135+
2136+
// Resolve credentials for a different user (routine context)
2137+
// Should fallback to "default" and find the token
2138+
let result = resolve_host_credentials(&caps, Some(&store), "routine_user_123", None).await;
2139+
2140+
assert!(!result.is_empty(), "fallback to default"); // safety: test code only
2141+
assert_eq!(result[0].secret_value, "global_token_value"); // safety: test code only
2142+
}
2143+
2144+
fn test_capabilities_with_google_oauth() -> Capabilities {
2145+
use crate::secrets::{CredentialLocation, CredentialMapping};
2146+
use crate::tools::wasm::capabilities::HttpCapability;
2147+
2148+
let mut creds = std::collections::HashMap::new();
2149+
creds.insert(
2150+
"google_oauth_token".to_string(),
2151+
CredentialMapping {
2152+
secret_name: "google_oauth_token".to_string(),
2153+
location: CredentialLocation::AuthorizationBearer,
2154+
host_patterns: vec!["sheets.googleapis.com".to_string()],
2155+
},
2156+
);
2157+
Capabilities {
2158+
http: Some(HttpCapability {
2159+
allowlist: vec![],
2160+
credentials: creds,
2161+
rate_limit: crate::tools::wasm::capabilities::RateLimitConfig::default(),
2162+
max_request_bytes: 1024 * 1024,
2163+
max_response_bytes: 10 * 1024 * 1024,
2164+
timeout: std::time::Duration::from_secs(30),
2165+
}),
2166+
..Default::default()
2167+
}
2168+
}
2169+
2170+
#[tokio::test]
2171+
async fn test_resolve_host_credentials_prefers_user_specific_over_default() {
2172+
use crate::secrets::SecretsStore;
2173+
use crate::tools::wasm::wrapper::resolve_host_credentials;
2174+
2175+
let store = test_secrets_store();
2176+
2177+
// Store token under "default" (global)
2178+
store
2179+
.create(
2180+
"default",
2181+
crate::secrets::CreateSecretParams::new("google_oauth_token", "global_token"),
2182+
)
2183+
.await
2184+
.expect("Failed to store global token"); // safety: test code only
2185+
2186+
// Store token under user_123 (user-specific)
2187+
store
2188+
.create(
2189+
"user_123",
2190+
crate::secrets::CreateSecretParams::new(
2191+
"google_oauth_token",
2192+
"user_specific_token",
2193+
),
2194+
)
2195+
.await
2196+
.expect("Failed to store user token"); // safety: test code only
2197+
2198+
// Create capabilities
2199+
let caps = test_capabilities_with_google_oauth();
2200+
2201+
// Resolve credentials for user_123
2202+
// Should prefer user_123's token over default
2203+
let result = resolve_host_credentials(&caps, Some(&store), "user_123", None).await;
2204+
2205+
assert!(!result.is_empty(), "has user credentials"); // safety: test code only
2206+
assert_eq!(result[0].secret_value, "user_specific_token", "user token"); // safety: test code only
2207+
}
2208+
2209+
#[tokio::test]
2210+
async fn test_resolve_host_credentials_no_fallback_when_already_default() {
2211+
use crate::secrets::SecretsStore;
2212+
use crate::tools::wasm::wrapper::resolve_host_credentials;
2213+
2214+
let store = test_secrets_store();
2215+
2216+
// Only store token under "default" (not a duplicate)
2217+
store
2218+
.create(
2219+
"default",
2220+
crate::secrets::CreateSecretParams::new("google_oauth_token", "default_token"),
2221+
)
2222+
.await
2223+
.expect("Failed to store default token"); // safety: test code only
2224+
2225+
// Create capabilities
2226+
let caps = test_capabilities_with_google_oauth();
2227+
2228+
// Resolve credentials for "default" user
2229+
// Should NOT attempt fallback (already looking up default)
2230+
let result = resolve_host_credentials(&caps, Some(&store), "default", None).await;
2231+
2232+
assert!(!result.is_empty(), "Should find default token"); // safety: test code only
2233+
assert_eq!(result[0].secret_value, "default_token"); // safety: test code only
2234+
}
2235+
2236+
#[tokio::test]
2237+
async fn test_resolve_host_credentials_missing_secret_warns() {
2238+
use crate::tools::wasm::wrapper::resolve_host_credentials;
2239+
2240+
let store = test_secrets_store();
2241+
2242+
// Don't store any token
2243+
2244+
// Create capabilities expecting a credential
2245+
let caps = test_capabilities_with_google_oauth();
2246+
2247+
// Resolve credentials when neither user nor default has the token
2248+
let result = resolve_host_credentials(&caps, Some(&store), "user_456", None).await;
2249+
2250+
// Should return empty since credential can't be found anywhere
2251+
assert!(result.is_empty(), "no credentials found"); // safety: test code only
2252+
}
20612253
}
14 KB
Binary file not shown.
8.92 KB
Binary file not shown.

tests/e2e/conftest.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ async def ironclaw_server(ironclaw_binary, mock_llm_server, wasm_tools_dir):
160160
"LIBSQL_PATH": os.path.join(_DB_TMPDIR.name, "e2e.db"),
161161
"SANDBOX_ENABLED": "false",
162162
"SKILLS_ENABLED": "true",
163-
"ROUTINES_ENABLED": "false",
163+
"ROUTINES_ENABLED": "true",
164164
"HEARTBEAT_ENABLED": "false",
165165
"EMBEDDING_ENABLED": "false",
166166
# WASM tool/channel support
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
Metadata-Version: 2.4
2+
Name: ironclaw-e2e
3+
Version: 0.1.0
4+
Requires-Python: >=3.11
5+
Requires-Dist: pytest>=8.0
6+
Requires-Dist: pytest-asyncio>=0.23
7+
Requires-Dist: pytest-playwright>=0.5
8+
Requires-Dist: pytest-timeout>=2.3
9+
Requires-Dist: playwright>=1.40
10+
Requires-Dist: aiohttp>=3.9
11+
Requires-Dist: httpx>=0.27
12+
Provides-Extra: vision
13+
Requires-Dist: anthropic>=0.40; extra == "vision"
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
README.md
2+
pyproject.toml
3+
ironclaw_e2e.egg-info/PKG-INFO
4+
ironclaw_e2e.egg-info/SOURCES.txt
5+
ironclaw_e2e.egg-info/dependency_links.txt
6+
ironclaw_e2e.egg-info/requires.txt
7+
ironclaw_e2e.egg-info/top_level.txt
8+
scenarios/__init__.py
9+
scenarios/test_chat.py
10+
scenarios/test_connection.py
11+
scenarios/test_csp.py
12+
scenarios/test_extension_oauth.py
13+
scenarios/test_extensions.py
14+
scenarios/test_html_injection.py
15+
scenarios/test_oauth_credential_fallback.py
16+
scenarios/test_pairing.py
17+
scenarios/test_routine_oauth_credential_injection.py
18+
scenarios/test_skills.py
19+
scenarios/test_sse_reconnect.py
20+
scenarios/test_tool_approval.py
21+
scenarios/test_tool_execution.py
22+
scenarios/test_wasm_lifecycle.py
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
pytest>=8.0
2+
pytest-asyncio>=0.23
3+
pytest-playwright>=0.5
4+
pytest-timeout>=2.3
5+
playwright>=1.40
6+
aiohttp>=3.9
7+
httpx>=0.27
8+
9+
[vision]
10+
anthropic>=0.40
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
scenarios

0 commit comments

Comments
 (0)