fix(R-1187 b): refuse a link whose parent path passes through a link; born keys listed; psql --set
gates / gates (push) Successful in 1m7s
gates / gates (push) Successful in 1m7s
Second automated security review finding, red-proved. The go-parity and docker -v gates that refused the push are green.
This commit is contained in:
1 parent
106325de43
commit
f24233c829
4 files changed
+70
-3
No files matched your search
@@ -37,6 +37,10 @@ hub answers 400 to both and nothing else changes.
|
|||||||
**Follow-up (the automated security review, same night):** the import writes only through `os.Root`, so no entry is
|
**Follow-up (the automated security review, same night):** the import writes only through `os.Root`, so no entry is
|
||||||
written through a link already in the folder that points outside; such an entry is refused and listed. Test
|
written through a link already in the folder that points outside; such an entry is refused and listed. Test
|
||||||
`TestR1187b_NoWriteThroughALinkThatLeavesTheFolder`, red-proved (the file landed outside with plain `os.Create`).
|
`TestR1187b_NoWriteThroughALinkThatLeavesTheFolder`, red-proved (the file landed outside with plain `os.Create`).
|
||||||
|
A second review finding: a link entry whose parent path passes through a link („a" → „.", then „a/b/l" → „../..")
|
||||||
|
passed the path-text check while really pointing above the folder — now refused and listed
|
||||||
|
(`TestR1187b_NoLinkThroughALinkedParent`, red-proved). psql takes `--set=ON_ERROR_STOP=1` (the `-v` short form tripped
|
||||||
|
the docker `-v` gate); the four new message keys are listed in `scripts/i18n_go_keys.json` as born keys.
|
||||||
- **R-1298:** the hold report skips an install's hold (`unhealthy_stop`, `update_failed`, `putback_pending`) of an app that
|
- **R-1298:** the hold report skips an install's hold (`unhealthy_stop`, `update_failed`, `putback_pending`) of an app that
|
||||||
is not installed (`holdreport.Reporter.SetInstalled`: the app's `app.yaml` exists — an unreadable one still counts,
|
is not installed (`holdreport.Reporter.SetInstalled`: the app's `app.yaml` exists — an unreadable one still counts,
|
||||||
R-1220) and reports it cleared if it was reported; a restore hold is always reported (R-379). 9202's four orphan holds
|
R-1220) and reports it cleared if it was reported; a restore hold is always reported (R-379). 9202's four orphan holds
|
||||||
|
|||||||
@@ -186,7 +186,7 @@ func TestR1187g_PostgisIsPostgresAndPsqlStopsOnError(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
args := strings.Join(psqlImportArgs("c", "u", "d"), " ")
|
args := strings.Join(psqlImportArgs("c", "u", "d"), " ")
|
||||||
if !strings.Contains(args, "-v ON_ERROR_STOP=1") {
|
if !strings.Contains(args, "--set=ON_ERROR_STOP=1") {
|
||||||
t.Fatalf("psql does not stop on error: %s", args)
|
t.Fatalf("psql does not stop on error: %s", args)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -216,3 +216,35 @@ func TestR1187b_NoWriteThroughALinkThatLeavesTheFolder(t *testing.T) {
|
|||||||
t.Fatal("a file was written OUTSIDE the app's folder through a pre-existing link")
|
t.Fatal("a file was written OUTSIDE the app's folder through a pre-existing link")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// (b), the security review's second case — a parser differential: link „a" → „." makes „a/b" really „b", so a later
|
||||||
|
// link „a/b/l" → „../.." passes a path-TEXT check (root/a/b/../.. = root) while it really points one level ABOVE the
|
||||||
|
// app's folder. A link entry whose parent path passes through a link is refused and listed. RED-PROOF: with the text
|
||||||
|
// check only, „a/b/l" is created and points outside.
|
||||||
|
func TestR1187b_NoLinkThroughALinkedParent(t *testing.T) {
|
||||||
|
dest := t.TempDir()
|
||||||
|
tarPath := filepath.Join(t.TempDir(), "d.tar")
|
||||||
|
f, _ := os.Create(tarPath)
|
||||||
|
tw := tar.NewWriter(f)
|
||||||
|
tw.WriteHeader(&tar.Header{Name: "a", Typeflag: tar.TypeSymlink, Linkname: "."})
|
||||||
|
tw.WriteHeader(&tar.Header{Name: "a/b/", Typeflag: tar.TypeDir, Mode: 0o755})
|
||||||
|
tw.WriteHeader(&tar.Header{Name: "a/b/l", Typeflag: tar.TypeSymlink, Linkname: "../.."})
|
||||||
|
tw.Close()
|
||||||
|
f.Close()
|
||||||
|
old := lchownFn
|
||||||
|
lchownFn = func(*os.Root, string, int, int) error { return nil }
|
||||||
|
defer func() { lchownFn = old }()
|
||||||
|
refused, err := extractTar(tarPath, dest)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
for _, p := range []string{filepath.Join(dest, "b", "l"), filepath.Join(dest, "a", "b", "l")} {
|
||||||
|
if _, err := os.Lstat(p); err == nil {
|
||||||
|
real, _ := filepath.EvalSymlinks(p)
|
||||||
|
t.Fatalf("a link through a linked parent was created (%s → %s)", p, real)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !strings.Contains(strings.Join(refused, ","), "a/b/l") {
|
||||||
|
t.Fatalf("the refused link must be listed, got %v", refused)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1087,7 +1087,7 @@ func waitForDB(containerID, dbType string, env map[string]string) error {
|
|||||||
func psqlImportArgs(containerID, user, dbName string) []string {
|
func psqlImportArgs(containerID, user, dbName string) []string {
|
||||||
// R-1187 (g): stop on the first error — the dump is --clean --if-exists, so a failing statement is a real failure,
|
// R-1187 (g): stop on the first error — the dump is --clean --if-exists, so a failing statement is a real failure,
|
||||||
// and without ON_ERROR_STOP psql went on and the import reported success over a half-loaded database.
|
// and without ON_ERROR_STOP psql went on and the import reported success over a half-loaded database.
|
||||||
return []string{"exec", "-i", containerID, "psql", "-v", "ON_ERROR_STOP=1", "-U", user, "-d", dbName}
|
return []string{"exec", "-i", containerID, "psql", "--set=ON_ERROR_STOP=1", "-U", user, "-d", dbName}
|
||||||
}
|
}
|
||||||
|
|
||||||
// importDBDump imports a (possibly gzipped) SQL dump into the running database container.
|
// importDBDump imports a (possibly gzipped) SQL dump into the running database container.
|
||||||
@@ -1203,6 +1203,26 @@ func extractTarGz(tgzPath, destDir string) error {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// parentThroughLink is true when any folder on name's path (inside the root) is a symbolic link.
|
||||||
|
func parentThroughLink(rt *os.Root, name string) bool {
|
||||||
|
dir := filepath.Dir(name)
|
||||||
|
if dir == "." {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
acc := ""
|
||||||
|
for _, part := range strings.Split(dir, string(os.PathSeparator)) {
|
||||||
|
acc = filepath.Join(acc, part)
|
||||||
|
fi, err := rt.Lstat(acc)
|
||||||
|
if err != nil {
|
||||||
|
return false // not there yet: MkdirAll creates real folders
|
||||||
|
}
|
||||||
|
if fi.Mode()&os.ModeSymlink != 0 {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
// lchownFn is the owner seam (a test records it; production is os.Lchown).
|
// lchownFn is the owner seam (a test records it; production is os.Lchown).
|
||||||
var lchownFn = func(r *os.Root, name string, uid, gid int) error { return r.Lchown(name, uid, gid) }
|
var lchownFn = func(r *os.Root, name string, uid, gid int) error { return r.Lchown(name, uid, gid) }
|
||||||
|
|
||||||
@@ -1270,6 +1290,13 @@ func extractTar(tarPath, destDir string) ([]string, error) {
|
|||||||
refused = append(refused, hdr.Name)
|
refused = append(refused, hdr.Name)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
// The text check below is exact only when no folder on the link's path is itself a link (a „a" → „." makes
|
||||||
|
// „a/b/l" really „b/l"): such an entry is refused (the security review's second case, pinned by
|
||||||
|
// TestR1187b_NoLinkThroughALinkedParent).
|
||||||
|
if parentThroughLink(rt, name) {
|
||||||
|
refused = append(refused, hdr.Name)
|
||||||
|
continue
|
||||||
|
}
|
||||||
resolved := filepath.Clean(filepath.Join(filepath.Dir(filepath.Join(root, name)), dst))
|
resolved := filepath.Clean(filepath.Join(filepath.Dir(filepath.Join(root, name)), dst))
|
||||||
if resolved != root && !strings.HasPrefix(resolved, root+string(os.PathSeparator)) {
|
if resolved != root && !strings.HasPrefix(resolved, root+string(os.PathSeparator)) {
|
||||||
refused = append(refused, hdr.Name)
|
refused = append(refused, hdr.Name)
|
||||||
|
|||||||
@@ -328,7 +328,11 @@
|
|||||||
"err.backup.restore_image_gone": "BORN AS A KEY (D48, operator ruling 2026-10-25; 09 decision 257) -- a restore downloads first: the refusal when an image of the backup's version cannot be downloaded (deleted by its maker, or the registry busy or unreachable); the app untouched. Pinned by internal/backup/restore_pull_test.go TestD48_*.",
|
"err.backup.restore_image_gone": "BORN AS A KEY (D48, operator ruling 2026-10-25; 09 decision 257) -- a restore downloads first: the refusal when an image of the backup's version cannot be downloaded (deleted by its maker, or the registry busy or unreachable); the app untouched. Pinned by internal/backup/restore_pull_test.go TestD48_*.",
|
||||||
"err.backup.restore_image_unreachable": "BORN AS A KEY (D48, operator ruling 2026-10-25; 09 decision 257) -- a restore downloads first: the refusal when an image of the backup's version cannot be downloaded (deleted by its maker, or the registry busy or unreachable); the app untouched. Pinned by internal/backup/restore_pull_test.go TestD48_*.",
|
"err.backup.restore_image_unreachable": "BORN AS A KEY (D48, operator ruling 2026-10-25; 09 decision 257) -- a restore downloads first: the refusal when an image of the backup's version cannot be downloaded (deleted by its maker, or the registry busy or unreachable); the app untouched. Pinned by internal/backup/restore_pull_test.go TestD48_*.",
|
||||||
"kept.backup.future_note": "BORN AS A KEY (R-1271, brief 2026-10-25 Part 4) -- a copy dated in the future is never the newest; when it is the only copy the kept-data question and the kept list say its date is in the future. Pinned by internal/backup/kept_pick_test.go and internal/api/r1271_future_copy_test.go.",
|
"kept.backup.future_note": "BORN AS A KEY (R-1271, brief 2026-10-25 Part 4) -- a copy dated in the future is never the newest; when it is the only copy the kept-data question and the kept list say its date is in the future. Pinned by internal/backup/kept_pick_test.go and internal/api/r1271_future_copy_test.go.",
|
||||||
"kept.choice.use.future": "BORN AS A KEY (R-1271, brief 2026-10-25 Part 4) -- a copy dated in the future is never the newest; when it is the only copy the kept-data question and the kept list say its date is in the future. Pinned by internal/backup/kept_pick_test.go and internal/api/r1271_future_copy_test.go."
|
"kept.choice.use.future": "BORN AS A KEY (R-1271, brief 2026-10-25 Part 4) -- a copy dated in the future is never the newest; when it is the only copy the kept-data question and the kept list say its date is in the future. Pinned by internal/backup/kept_pick_test.go and internal/api/r1271_future_copy_test.go.",
|
||||||
|
"api.escrow.recovery_offer_open": "BORN AS A KEY (R-1219, brief 2026-10-26 night Part 3) -- the escrow start handler refuses during a recovery offer. Pinned by TestR1219_NoCeremonyDuringARecoveryOffer.",
|
||||||
|
"err.appexport.kulonleges_fajl": "BORN AS A KEY (R-1187 a, brief 2026-10-26 night Part 4) -- the export refuses a FIFO, socket or device, naming the path. Pinned by TestR1187a_AFifoFailsTheExportNamingThePath.",
|
||||||
|
"err.appexport.import_foglalt": "BORN AS A KEY (R-1187 d, brief 2026-10-26 night Part 4) -- an import is refused while another operation holds the box. Pinned by TestR1187d_AnImportWaitsForARestore.",
|
||||||
|
"err.appexport.hdd_path_hibas": "BORN AS A KEY (R-1187 f, brief 2026-10-26 night Part 4) -- a bundle with an empty or relative HDD_PATH is refused before anything is touched. Pinned by TestR1187f_AnEmptyOrRelativeHDDPathIsRefused."
|
||||||
},
|
},
|
||||||
"flash.share.already_on": "A megosztás már be van kapcsolva.",
|
"flash.share.already_on": "A megosztás már be van kapcsolva.",
|
||||||
"flash.share.enable_failed": "A megosztás bekapcsolása nem sikerült.",
|
"flash.share.enable_failed": "A megosztás bekapcsolása nem sikerült.",
|
||||||
|
|||||||
Reference in new issue
Block a user