Skip to content

Commit eb50315

Browse files
authored
ggladstone/eas 4431 extend (#91)
* feat(reporter): surface manifest parse failures in FSRT output Previously, manifest deserialization errors were silently swallowed via unwrap_or_default(), causing FSRT to scan an empty manifest and return zero findings with errors: false. Change get_manifest() to return Result<ForgeManifest, serde_yaml::Error> and handle parse failures in scan_directory() by returning a structured error report with errors: true and a detailed error_message field, e.g.: 'Could not process manifest, failed on modules.scheduledTrigger[0].interval: unknown variant `fiveMinute`, expected one of `hour`, `day`, `week` at line 27 column 17' Add error_message: String field (default empty) to Report. Add Report::error() constructor, has_errors() and error_message() accessors. * fix(manifest): support endpoint-routed triggers Allow modules.trigger entries to deserialize with endpoint instead of function, matching Forge manifest behavior where endpoint is required when function is omitted. This prevents valid endpoint-routed lifecycle triggers from failing manifest processing with missing field function. * fix(manifest): support boolean macro config Allow Confluence macro modules to deserialize config: true in addition to config objects with function handlers. This prevents valid macro manifests from failing manifest processing. * fix(manifest): default missing permissions to empty scopes Allow manifests without a permissions block to deserialize by defaulting permissions to Perms::default(), equivalent to permissions.scopes: []. This prevents apps that omit permissions from failing manifest processing before scanning. * fix(manifest): support i18n jira admin page titles Allow jira:adminPage title to deserialize from either a plain string or an i18n object. This prevents valid manifests using title.i18n from failing manifest processing with invalid type map * fix(manifest): own rovo agent descriptions Deserialize rovo:agent description as an owned String instead of a borrowed str so YAML strings requiring escape processing, such as descriptions containing newlines, do not fail manifest processing. This prevents valid Rovo agent manifests from failing with expected a borrowed string. * fix(manifest): own rovo agent text fields Deserialize rovo:agent description and conversationStarters as owned Strings instead of borrowed strs. YAML strings containing escape sequences such as \n require allocation during deserialization, so borrowed strs can fail manifest processing with expected a borrowed string. Owning these fields allows valid Rovo agent manifests with multiline or escaped text to parse correctly. * fix(manifest): resources[].path string processing cannot be a borrowed string * chore: fmt isssues
1 parent b79668d commit eb50315

5 files changed

Lines changed: 111 additions & 37 deletions

File tree

crates/forge_analyzer/src/reporter.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ pub struct Report {
4747
ended_at: OffsetDateTime,
4848
scanned: Vec<String>,
4949
errors: bool,
50+
error_message: String,
5051
}
5152

5253
pub struct Reporter {
@@ -106,6 +107,7 @@ impl Reporter {
106107
ended_at: OffsetDateTime::now_utc(),
107108
scanned: self.apps.into_iter().map(|(key, _)| key).collect(),
108109
errors: false,
110+
error_message: String::new(),
109111
}
110112
}
111113

@@ -115,10 +117,34 @@ impl Reporter {
115117
}
116118

117119
impl Report {
120+
#[inline]
121+
pub fn error(error_message: String, scanned: Vec<String>) -> Self {
122+
let now = OffsetDateTime::now_utc();
123+
Self {
124+
vulns: Vec::new(),
125+
scanner: "FSRT",
126+
started_at: now,
127+
ended_at: now,
128+
scanned,
129+
errors: true,
130+
error_message,
131+
}
132+
}
133+
118134
#[inline]
119135
pub fn into_vulns(&self) -> &[Vulnerability] {
120136
&self.vulns
121137
}
138+
139+
#[inline]
140+
pub fn has_errors(&self) -> bool {
141+
self.errors
142+
}
143+
144+
#[inline]
145+
pub fn error_message(&self) -> &str {
146+
&self.error_message
147+
}
122148
}
123149

124150
impl Default for Reporter {

crates/forge_loader/src/manifest.rs

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,11 @@ struct RawTrigger<'a> {
131131
// maps to Trigger under Common Modules
132132
#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
133133
struct EventTrigger<'a> {
134-
#[serde(flatten, borrow)]
135-
raw: RawTrigger<'a>,
134+
key: &'a str,
135+
#[serde(default, borrow)]
136+
function: Option<&'a str>,
137+
#[serde(default, borrow)]
138+
endpoint: Option<&'a str>,
136139
#[serde(borrow)]
137140
events: Vec<&'a str>,
138141
}
@@ -184,18 +187,42 @@ struct ContentByLineItem<'a> {
184187
dynamic_properties: JustFunc<'a>,
185188
}
186189

190+
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Copy)]
191+
#[serde(untagged)]
192+
#[serde(bound(deserialize = "'de: 'a"))]
193+
enum MacroConfig<'a> {
194+
Enabled(bool),
195+
Object(JustFunc<'a>),
196+
}
197+
198+
impl<'a> HasFunctions<'a> for MacroConfig<'a> {
199+
fn append_functions<I: Extend<&'a str>>(&self, funcs: &mut I) {
200+
if let Self::Object(config) = self {
201+
config.append_functions(funcs);
202+
}
203+
}
204+
}
205+
187206
#[derive(Default, Debug, Clone, PartialEq, Eq, Deserialize, Copy)]
188207
pub struct MacroMod<'a> {
189208
#[serde(flatten, borrow)]
190209
common_keys: CommonKey<'a>,
191-
config: Option<JustFunc<'a>>,
210+
config: Option<MacroConfig<'a>>,
192211
export: Option<JustFunc<'a>>,
193212
}
194213

195214
// Jira Modules
215+
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Copy)]
216+
#[serde(untagged)]
217+
#[serde(bound(deserialize = "'de: 'a"))]
218+
enum LocalizedText<'a> {
219+
Text(&'a str),
220+
I18n { i18n: &'a str },
221+
}
222+
196223
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Copy)]
197224
pub struct JiraAdminPage<'a> {
198-
title: &'a str,
225+
title: LocalizedText<'a>,
199226
#[serde(flatten, borrow)]
200227
common_keys: CommonKey<'a>,
201228
}
@@ -267,15 +294,15 @@ struct AssetsImportType<'a> {
267294
pub struct RovoAgent<'a> {
268295
pub key: &'a str,
269296
pub name: &'a str,
270-
pub description: Option<&'a str>,
297+
pub description: Option<String>,
271298
pub icon: Option<&'a str>,
272299
pub prompt: String, // as may be multiline
273-
#[serde(default, rename = "conversationStarters", borrow)]
274-
pub conversation_starters: Vec<&'a str>,
300+
#[serde(default, rename = "conversationStarters")]
301+
pub conversation_starters: Vec<String>,
275302
#[serde(default, borrow)]
276303
pub actions: Vec<&'a str>,
277304
#[serde(rename = "followUpPrompt")]
278-
pub follow_up_prompt: Option<&'a str>,
305+
pub follow_up_prompt: Option<String>,
279306
}
280307

281308
#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
@@ -638,10 +665,10 @@ pub struct Module<'a> {
638665
extra: FxHashMap<String, serde_yaml::Value>,
639666
}
640667

641-
#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, Deserialize)]
668+
#[derive(Default, Debug, Clone, PartialEq, Eq, Deserialize)]
642669
pub struct Resource<'a> {
643670
pub key: &'a str,
644-
pub path: &'a str,
671+
pub path: String,
645672
}
646673

647674
/// The representation of a Forge app's `manifest.yml`
@@ -655,7 +682,7 @@ pub struct ForgeManifest<'a> {
655682
pub app: AppInfo<'a>,
656683
#[serde(borrow)]
657684
pub modules: ForgeModules<'a>,
658-
#[serde(borrow)]
685+
#[serde(default, borrow)]
659686
pub permissions: Perms<'a>,
660687
pub remotes: Option<Vec<Remotes>>,
661688
#[serde(default, borrow)]
@@ -1169,9 +1196,11 @@ mod tests {
11691196
if let Some(string) = resolver.function {
11701197
assert_eq!(string, "Catch-me-if-you-can1");
11711198
}
1172-
if let Some(justfunc) = manifest.modules.macros[0].config {
1199+
if let Some(MacroConfig::Object(justfunc)) = manifest.modules.macros[0].config {
11731200
let func = justfunc.function.unwrap();
11741201
assert_eq!(func, "Catch-me-if-you-can2");
1202+
} else {
1203+
panic!("No config function found")
11751204
}
11761205

11771206
if let Some(justfunc) = manifest.modules.macros[0].export {
@@ -1327,7 +1356,7 @@ mod tests {
13271356
let agent = &manifest.modules.rovo_agent[0];
13281357
assert_eq!(agent.key, "data-discoverability");
13291358
assert_eq!(agent.name, "Data Discoverability");
1330-
assert_eq!(agent.description, Some("Test description"));
1359+
assert_eq!(agent.description.as_deref(), Some("Test description"));
13311360
assert_eq!(
13321361
agent.prompt,
13331362
"You are a helpful assistant that helps users manage their project risks. \nYou can retrieve risks from the risk register, create new risks and update existing ones."

crates/fsrt/src/forge_project.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use forge_analyzer::ctx::AppCtx;
33
use forge_analyzer::definitions::{Environment, PackageData, run_resolver};
44
use forge_loader::manifest::{Entrypoint, ForgeManifest, Resolved};
55
use forge_permission_resolver::permissions_resolver::PermMap;
6+
use serde_yaml::Error as YamlError;
67
use std::collections::HashSet;
78
use std::path::{Path, PathBuf};
89
use std::sync::Arc;
@@ -78,7 +79,7 @@ pub(crate) trait ForgeProjectTrait<'a> {
7879
#[allow(dead_code)]
7980
fn get_secret_packages(&self) -> Vec<PackageData>;
8081

81-
fn get_manifest(&self) -> ForgeManifest<'_>;
82+
fn get_manifest(&self) -> Result<ForgeManifest<'_>, YamlError>;
8283
}
8384
pub(crate) struct ForgeProject<'a> {
8485
#[allow(dead_code)]
@@ -135,8 +136,7 @@ impl ForgeProjectTrait<'_> for ForgeProjectFromDir {
135136
}
136137
}
137138

138-
fn get_manifest(&self) -> ForgeManifest<'_> {
139-
let out = serde_yaml::from_str(&self.manifest_file_content);
140-
out.unwrap_or_default()
139+
fn get_manifest(&self) -> Result<ForgeManifest<'_>, YamlError> {
140+
serde_yaml::from_str(&self.manifest_file_content)
141141
}
142142
}

crates/fsrt/src/main.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,19 @@ pub(crate) fn scan_directory<'a>(
372372
secret_packages: &[PackageData],
373373
) -> Result<Report> {
374374
let paths = project.get_paths();
375-
let manifest = project.get_manifest();
375+
let manifest = match project.get_manifest() {
376+
Ok(manifest) => manifest,
377+
Err(err) => {
378+
let error_message = format!(
379+
"Could not process manifest, failed on {err}, submit an issue if you believe this is a bug --> https://github.com/atlassian-labs/FSRT/issues"
380+
);
381+
warn!("{error_message}");
382+
return Ok(Report::error(
383+
error_message,
384+
vec![opts.appkey.clone().unwrap_or_default()],
385+
));
386+
}
387+
};
376388
let requested_permissions = manifest.permissions;
377389
let permissions_declared = requested_permissions
378390
.scopes

crates/fsrt/src/test.rs

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ impl ReportExt for Report {
107107
pub(crate) struct MockForgeProject<'a> {
108108
pub files_name_to_source: HashMap<PathBuf, Arc<SourceFile>>,
109109
pub test_manifest: ForgeManifest<'a>,
110+
pub manifest_file_content: Option<String>,
110111
pub cm: Lrc<SourceMap>,
111112
}
112113

@@ -121,28 +122,28 @@ impl<'a> MockForgeProject<'a> {
121122
pub fn files_from_string(string: &'a str) -> Self {
122123
let different_files = string.split("//").filter(|file| !file.is_empty());
123124

124-
let manifest = if let Some(manifest_string) = different_files.clone().find(|string| {
125-
string
126-
.replace("//", "")
127-
.trim_start()
128-
.starts_with("manifest.yaml")
129-
|| string
130-
.trim_start()
125+
let manifest_file_content = different_files
126+
.clone()
127+
.find(|string| {
128+
string
131129
.replace("//", "")
132-
.starts_with("manifest.yml")
133-
}) {
134-
serde_yaml::from_str(manifest_string.split_once('\n').unwrap().1).unwrap_or_default()
135-
} else {
136-
ForgeManifest::create_manifest_with_func_mod(FunctionMod {
137-
key: "main",
138-
handler: "index.run",
139-
providers: None,
130+
.trim_start()
131+
.starts_with("manifest.yaml")
132+
|| string
133+
.trim_start()
134+
.replace("//", "")
135+
.starts_with("manifest.yml")
140136
})
141-
};
137+
.map(|manifest_string| manifest_string.split_once('\n').unwrap().1.to_string());
142138

143139
let mut mock_forge_project = MockForgeProject {
144140
files_name_to_source: HashMap::new(),
145-
test_manifest: manifest.to_owned(),
141+
test_manifest: ForgeManifest::create_manifest_with_func_mod(FunctionMod {
142+
key: "main",
143+
handler: "index.run",
144+
providers: None,
145+
}),
146+
manifest_file_content,
146147
cm: Arc::default(),
147148
};
148149

@@ -186,8 +187,12 @@ impl<'a> ForgeProjectTrait<'a> for MockForgeProject<'a> {
186187
vec![]
187188
}
188189

189-
fn get_manifest(&self) -> ForgeManifest<'_> {
190-
self.test_manifest.clone()
190+
fn get_manifest(&self) -> Result<ForgeManifest<'_>, serde_yaml::Error> {
191+
if let Some(manifest_file_content) = &self.manifest_file_content {
192+
serde_yaml::from_str(manifest_file_content)
193+
} else {
194+
Ok(self.test_manifest.clone())
195+
}
191196
}
192197
}
193198

@@ -224,6 +229,7 @@ fn test_simple() {
224229
let mut test_forge_project = MockForgeProject {
225230
test_manifest: forge_manifest,
226231
files_name_to_source: HashMap::new(),
232+
manifest_file_content: None,
227233
cm: Lrc::new(SourceMap::default()),
228234
};
229235
test_forge_project.add_file(
@@ -248,6 +254,7 @@ fn test_secret_vuln() {
248254
let mut test_forge_project = MockForgeProject {
249255
test_manifest: forge_manifest,
250256
files_name_to_source: HashMap::new(),
257+
manifest_file_content: None,
251258
cm: Lrc::new(SourceMap::default()),
252259
};
253260
test_forge_project.add_file(

0 commit comments

Comments
 (0)