Skip to content

Commit bbe47d6

Browse files
hyperpolymathhyperpolymath
committed
fix(recon-silly-ation): eliminate Obj.magic, getUnsafe, getExn partial patterns
Addresses the 17 concrete dangerous-pattern findings from panic-attack assail that I flagged in the de-gitlink commit (87995d9). All changes are mechanical substitutions; no semantics touched. src/ArangoClient.res (12 fixes): - Tighten 5 @send externals (save/document/update/query/all) from `{..}` to `Js.Json.t`. Loose type was forcing Obj.magic at every call site. - Remove 10 `->Obj.magic` casts (banned pattern per memory rule); use Js.Json.object_(dict) to build bind-var JSON where needed. - Replace 2 `Belt.Array.getUnsafe(arr, i)` inside bounded for-loops with `switch Belt.Array.get(arr, i)` pattern match. src/Pipeline.res (2 fixes): - Replace 2 `Belt.Array.getUnsafe` in async store-conflict/store-resolution loops with the same Belt.Array.get pattern match. src/ConflictResolver.res (2 fixes): - Replace 2 `Belt.Array.getUnsafe(arr, 0)` inside Belt.Array.every callbacks with extract-first idiom using Belt.Array.get. Handles the empty-array case as vacuously true. src/HaskellBridge.res (1 fix): - Replace `Belt.Option.getExn` on decodeObject result with keepMap + Option pattern match; array elements that aren't JSON objects are now skipped instead of crashing the whole validation pass. NOT touched (intentional): - try/catch FFI-boundary blocks. These are the correct ReScript idiom for converting JS exceptions (arangojs promises, node crypto, child_process) into `result<'a, string>` at the FFI edge. panic-attack still reports "unsafe_blocks: 13" after this commit because it conflates try/catch with partial casts under the same category. - Test-file unwraps (tests/*Test.res) — expected-case assertions. - Js.Exn.raiseError in SecurityScheme.res — marks "not yet implemented" fallback paths (Ed448/SPHINCS+ WASM bindings). Not built/tested locally: repo has no node_modules and no ReScript toolchain wired through the Justfile, so rescript build cannot run here. Diffs are pure substitutions verified against the ReScript Belt/Js.Json API surface; any type mismatches will surface at the next build attempt.
1 parent 87995d9 commit bbe47d6

4 files changed

Lines changed: 72 additions & 49 deletions

File tree

‎recon-silly-ation/src/ArangoClient.res‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,17 @@ external createDatabase: {..} => database = "Database"
1717
@send external useDatabase: (database, string) => database = "useDatabase"
1818
@send external collection: (database, string) => collection = "collection"
1919
@send external graph: (database, string) => graph = "graph"
20-
@send external query: (database, string, {..}) => promise<aqlQuery> = "query"
20+
// Typed externals: arangojs accepts and returns plain JSON, so Js.Json.t is the
21+
// correct ReScript type. Using `{..}` here previously forced Obj.magic at every
22+
// call site, which panic-attack flagged as unsafe_blocks.
23+
@send external query: (database, string, Js.Json.t) => promise<aqlQuery> = "query"
2124

22-
@send external save: (collection, {..}) => promise<{..}> = "save"
23-
@send external document: (collection, string) => promise<{..}> = "document"
24-
@send external update: (collection, string, {..}) => promise<{..}> = "update"
25-
@send external remove: (collection, string) => promise<{..}> = "remove"
25+
@send external save: (collection, Js.Json.t) => promise<Js.Json.t> = "save"
26+
@send external document: (collection, string) => promise<Js.Json.t> = "document"
27+
@send external update: (collection, string, Js.Json.t) => promise<Js.Json.t> = "update"
28+
@send external remove: (collection, string) => promise<Js.Json.t> = "remove"
2629

27-
@send external all: aqlQuery => promise<array<{..}>> = "all"
30+
@send external all: aqlQuery => promise<array<Js.Json.t>> = "all"
2831

2932
// Client state
3033
type client = {
@@ -119,7 +122,7 @@ let edgeToJson = (edge: edge): Js.Json.t => {
119122
let insertDocument = async (client: client, doc: document): result<unit, string> => {
120123
try {
121124
let json = documentToJson(doc)
122-
let _ = await client.collections.documents->save(json->Obj.magic)
125+
let _ = await client.collections.documents->save(json)
123126
Ok()
124127
} catch {
125128
| exn =>
@@ -136,9 +139,12 @@ let insertDocuments = async (
136139
): result<unit, string> => {
137140
let results = []
138141
for i in 0 to Belt.Array.length(documents) - 1 {
139-
let doc = Belt.Array.getUnsafe(documents, i)
140-
let result = await insertDocument(client, doc)
141-
results->Js.Array2.push(result)->ignore
142+
switch Belt.Array.get(documents, i) {
143+
| Some(doc) =>
144+
let result = await insertDocument(client, doc)
145+
results->Js.Array2.push(result)->ignore
146+
| None => ()
147+
}
142148
}
143149

144150
let errors =
@@ -160,7 +166,7 @@ let insertDocuments = async (
160166
let insertEdge = async (client: client, edge: edge): result<unit, string> => {
161167
try {
162168
let json = edgeToJson(edge)
163-
let _ = await client.edges.relationships->save(json->Obj.magic)
169+
let _ = await client.edges.relationships->save(json)
164170
Ok()
165171
} catch {
166172
| exn =>
@@ -172,9 +178,12 @@ let insertEdge = async (client: client, edge: edge): result<unit, string> => {
172178
let insertEdges = async (client: client, edges: array<edge>): result<unit, string> => {
173179
let results = []
174180
for i in 0 to Belt.Array.length(edges) - 1 {
175-
let edge = Belt.Array.getUnsafe(edges, i)
176-
let result = await insertEdge(client, edge)
177-
results->Js.Array2.push(result)->ignore
181+
switch Belt.Array.get(edges, i) {
182+
| Some(edge) =>
183+
let result = await insertEdge(client, edge)
184+
results->Js.Array2.push(result)->ignore
185+
| None => ()
186+
}
178187
}
179188

180189
let errors =
@@ -211,7 +220,7 @@ let storeConflict = async (client: client, conflict: conflict): result<unit, str
211220
]),
212221
)
213222

214-
let _ = await client.collections.conflicts->save(json->Obj.magic)
223+
let _ = await client.collections.conflicts->save(json)
215224
Ok()
216225
} catch {
217226
| exn =>
@@ -249,7 +258,7 @@ let storeResolution = async (
249258
]),
250259
)
251260

252-
let _ = await client.collections.resolutions->save(json->Obj.magic)
261+
let _ = await client.collections.resolutions->save(json)
253262
Ok()
254263
} catch {
255264
| exn =>
@@ -266,7 +275,7 @@ let findDocumentByHash = async (
266275
): result<option<Js.Json.t>, string> => {
267276
try {
268277
let result = await client.collections.documents->document(hash)
269-
Ok(Some(result->Obj.magic))
278+
Ok(Some(result))
270279
} catch {
271280
| _ => Ok(None) // Document not found
272281
}
@@ -281,9 +290,9 @@ let findDuplicates = async (client: client): result<array<Js.Json.t>, string> =>
281290
FILTER count > 1
282291
RETURN { hash, count }
283292
`
284-
let result = await client.db->query(aql, Js.Dict.empty()->Obj.magic)
293+
let result = await client.db->query(aql, Js.Json.object_(Js.Dict.empty()))
285294
let data = await result->all
286-
Ok(data->Obj.magic)
295+
Ok(data)
287296
} catch {
288297
| exn =>
289298
Error(
@@ -303,9 +312,9 @@ let findRelatedDocuments = async (
303312
RETURN { vertex: v, edge: e, path: p }
304313
`
305314
let bindVars = Js.Dict.fromArray([("start", Js.Json.string("documents/" ++ hash))])
306-
let result = await client.db->query(aql, bindVars->Obj.magic)
315+
let result = await client.db->query(aql, Js.Json.object_(bindVars))
307316
let data = await result->all
308-
Ok(data->Obj.magic)
317+
Ok(data)
309318
} catch {
310319
| exn =>
311320
Error(
@@ -318,7 +327,7 @@ let findRelatedDocuments = async (
318327
let healthCheck = async (client: client): result<bool, string> => {
319328
try {
320329
let aql = "RETURN 1"
321-
let _ = await client.db->query(aql, Js.Dict.empty()->Obj.magic)
330+
let _ = await client.db->query(aql, Js.Json.object_(Js.Dict.empty()))
322331
Ok(true)
323332
} catch {
324333
| exn =>

‎recon-silly-ation/src/ConflictResolver.res‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,10 @@ let detectConflicts = (documents: array<document>): array<conflict> => {
205205
if Belt.Array.length(docs) > 1 {
206206
// Multiple documents with same hash but different paths
207207
let paths = docs->Belt.Array.map(d => d.metadata.path)
208-
let allSamePath = paths->Belt.Array.every(p => p == Belt.Array.getUnsafe(paths, 0))
208+
let allSamePath = switch Belt.Array.get(paths, 0) {
209+
| None => true // vacuously — no paths means no divergence
210+
| Some(first) => paths->Belt.Array.every(p => p == first)
211+
}
209212

210213
if !allSamePath {
211214
conflicts
@@ -238,9 +241,10 @@ let detectConflicts = (documents: array<document>): array<conflict> => {
238241
// Check if versions differ
239242
let versions = docs->Belt.Array.keepMap(d => d.metadata.version)
240243
if Belt.Array.length(versions) > 1 {
241-
let allSame = versions->Belt.Array.every(v => {
242-
compareVersions(v, Belt.Array.getUnsafe(versions, 0)) == 0
243-
})
244+
let allSame = switch Belt.Array.get(versions, 0) {
245+
| None => true
246+
| Some(first) => versions->Belt.Array.every(v => compareVersions(v, first) == 0)
247+
}
244248

245249
if !allSame {
246250
conflicts

‎recon-silly-ation/src/HaskellBridge.res‎

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -61,24 +61,28 @@ let validateDocument = (
6161
->Belt.Option.flatMap(Js.Json.decodeArray)
6262
->Belt.Option.getWithDefault([])
6363

64+
// Drop any array element that isn't a JSON object instead of crashing on
65+
// the first bad one. keepMap returns Some values and skips Nones.
6466
let parsedViolations =
65-
violations->Belt.Array.map(v => {
66-
let obj = v->Js.Json.decodeObject->Belt.Option.getExn
67-
68-
{
69-
violationField: obj
70-
->Js.Dict.get("violationField")
71-
->Belt.Option.flatMap(Js.Json.decodeString)
72-
->Belt.Option.getWithDefault(""),
73-
violationMessage: obj
74-
->Js.Dict.get("violationMessage")
75-
->Belt.Option.flatMap(Js.Json.decodeString)
76-
->Belt.Option.getWithDefault(""),
77-
violationSeverity: obj
78-
->Js.Dict.get("violationSeverity")
79-
->Belt.Option.flatMap(Js.Json.decodeString)
80-
->Belt.Option.getWithDefault("warning"),
81-
violationLine: None,
67+
violations->Belt.Array.keepMap(v => {
68+
switch v->Js.Json.decodeObject {
69+
| None => None
70+
| Some(obj) =>
71+
Some({
72+
violationField: obj
73+
->Js.Dict.get("violationField")
74+
->Belt.Option.flatMap(Js.Json.decodeString)
75+
->Belt.Option.getWithDefault(""),
76+
violationMessage: obj
77+
->Js.Dict.get("violationMessage")
78+
->Belt.Option.flatMap(Js.Json.decodeString)
79+
->Belt.Option.getWithDefault(""),
80+
violationSeverity: obj
81+
->Js.Dict.get("violationSeverity")
82+
->Belt.Option.flatMap(Js.Json.decodeString)
83+
->Belt.Option.getWithDefault("warning"),
84+
violationLine: None,
85+
})
8286
}
8387
})
8488

‎recon-silly-ation/src/Pipeline.res‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,9 +263,12 @@ let executeStage = async (
263263
| None => ()
264264
| Some(c) => {
265265
for i in 0 to Belt.Array.length(conflicts) - 1 {
266-
let conflict = Belt.Array.getUnsafe(conflicts, i)
267-
let _ = await ArangoClient.storeConflict(c, conflict)
268-
()
266+
switch Belt.Array.get(conflicts, i) {
267+
| Some(conflict) =>
268+
let _ = await ArangoClient.storeConflict(c, conflict)
269+
()
270+
| None => ()
271+
}
269272
}
270273
}
271274
}
@@ -297,9 +300,12 @@ let executeStage = async (
297300
| None => ()
298301
| Some(c) => {
299302
for i in 0 to Belt.Array.length(resolutions) - 1 {
300-
let resolution = Belt.Array.getUnsafe(resolutions, i)
301-
let _ = await ArangoClient.storeResolution(c, resolution)
302-
()
303+
switch Belt.Array.get(resolutions, i) {
304+
| Some(resolution) =>
305+
let _ = await ArangoClient.storeResolution(c, resolution)
306+
()
307+
| None => ()
308+
}
303309
}
304310

305311
// Create superseded edges

0 commit comments

Comments
 (0)